-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor dashboard #548
Refactor dashboard #548
Conversation
Hi @laurasootes Would you be willing to have a look at this and review it?
|
Are you in the right branch? |
never mind, I did not pull all changes yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good! The general outline already looks much nicer than the dash version. Also a lot less code, which makes it way more readable.
Some suggestions:
- I do get a whole lot of prints while running the dashboard for kernelshap, but can't find where it originates from (np.sum(w_aug), np.sum(self.kernelWeights) =, phi =). If you do know, can you remove it?
- Could you move all data to the data folder and remove the old app_data? (or the other way around)
I do have some aesthetic suggestions, but I think we can start from here and have another session with some people to decide how want the exact layout to look like.
One thing is that the colours for resnet don't look right yet and you can't see the original figure anymore, but I don't think that has to be fixed in this PR, since it is coming from the Dianna visualisation).
Thanks for the review. I checked the data directory and I think that should all be OK. You may have existing data in the Let's create a new issue to follow up on the remaining points. |
This PR refactors the dashboard and moves it to streamlit.
Closes #542
In addition:
Resolves #418 (caching is handled by st.cache)
Resolves #428 (stop button top right)
Resolves #511 (no longer necessary)
Resolves #521 (no longer relevant, dashboard auto-updates)
Resolves #525 (methods are in their own containers)
Todo