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

378 update button for execution #396

Merged
merged 13 commits into from
Dec 7, 2022
Merged

Conversation

geek-yang
Copy link
Member

Implement an update button for the dashboard to run the given XAI methods with provided model and inputs.

This PR solves the rapid refreshing problem that occurs in PR #387 and it closes #378.

@geek-yang
Copy link
Member Author

Note: this PR should be merged after PR #387 is merged to main. Before merging this PR, merge the main branch first to include the latest updates from PR #387.

@geek-yang geek-yang marked this pull request as ready for review November 29, 2022 16:37
@laurasootes
Copy link
Contributor

@geek-yang & @cpranav93
The update button looks good! It also seem to work well. I only have a few small suggestions:

  • If I set the settings and click the update button, the calculations start to do their work as they should. However, if during the calculations I want to change any settings, I have to wait for the calculation to finish to be able to update it again. For RESNET this takes quite some time. Would there be an easy way to fix this? One could of course also simply refresh the dashboard to do this, so it's not neccisarily a problem.
  • It is good to see that the error message works. I will have a look if it still works in Dashboard: selection method for XAI method #386 when I pull this into that PR, since it seems I broke the old error message :)
  • The error message appears very close to the components above it. I would suggest to update html.Div layout settings for the error message to add some space. That would also make the error stand out more. (line 354 in callbacks)
  • I am wondering if it would look better if the update button would look better if located at the far right of the screen. Also maybe circular? It might stand out more like that. But maybe Jesus might have an opinion on that based on his experience with UI (we are planning to consult him on the looks of the dashboard as a whole)
  • Another layout thing: maybe the update button is located a bit close to the "XAI method specific settings" header? Maybe it would be nice to have a bit more space between them (but of course this is no longer relevant is we decide to move the button to the right).

return html.Div(['']), utilities.blank_fig()
if (not sel_methods):
return html.Div(['']), utilities.blank_fig()
# if ((ctx.triggered[0]["prop_id"] == "upload-model-img.filename") or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this part is no longer needed, it can maybe be deleted?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this part relates to the global cache in #418. So I do not want to delete it yet.

@cpranav93
Copy link
Contributor

I have modified the layout to account fo @laurasootes's comments. I tried moving the button to the right but then I feel that the length of the window is too large - I would not recommend it. We may anyway redo the whole layout based on jesus' comments. Also, a round button is apparently complicated in dash! Who would have thought!

I have now also added a stop explanation button which will stop the loading in the dashboard - however the actual computation in the background still continues to go on. I am in the process of trying to figure ths out but I think this warrants a new issue in itself. So if you agree and are happy with this PR, we can merge it and I can create a new issue.

@cpranav93 cpranav93 merged commit 1d72c0d into main Dec 7, 2022
@elboyran elboyran deleted the 378-update-button-for-execution branch March 21, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refresh dashboard for every change of image/model/XAI method.
3 participants