Skip to content
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

Merged
merged 22 commits into from
Apr 11, 2023
Merged

Refactor dashboard #548

merged 22 commits into from
Apr 11, 2023

Conversation

stefsmeets
Copy link
Contributor

@stefsmeets stefsmeets commented Apr 6, 2023

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

  • Sort kernelshap issue
  • Organize model prepare step

@stefsmeets stefsmeets marked this pull request as ready for review April 6, 2023 14:52
@stefsmeets
Copy link
Contributor Author

Hi @laurasootes Would you be willing to have a look at this and review it?

pip install streamlit
git checkout dashboard --
cd dashboard
streamlit run dashboard.py

@laurasootes
Copy link
Contributor

laurasootes commented Apr 11, 2023

I get an error message when trying to start the dashboard :( (using python 3.9.10, no problems installing streamlit)

image

@stefsmeets
Copy link
Contributor Author

Are you in the right branch? app.run_server(debug=True) is the way the old dashboard got started. There is no such code in this branch 😅

@laurasootes
Copy link
Contributor

never mind, I did not pull all changes yet.

Copy link
Contributor

@laurasootes laurasootes left a 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).

@stefsmeets
Copy link
Contributor Author

stefsmeets commented Apr 11, 2023

Thanks for the review. I checked the data directory and I think that should all be OK. You may have existing data in the app_data directory, so that git did not remove it when you pulled the branch.

Let's create a new issue to follow up on the remaining points.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants