-
Notifications
You must be signed in to change notification settings - Fork 19
Add decision and waterfall plots to rsmexplain #603
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
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #603 +/- ##
==========================================
+ Coverage 93.18% 93.21% +0.03%
==========================================
Files 32 32
Lines 4842 4851 +9
==========================================
+ Hits 4512 4522 +10
+ Misses 330 329 -1
☔ View full report in Codecov by Sentry. |
dc18995 to
ecccc33
Compare
ecccc33 to
8878bc9
Compare
desilinguist
left a comment
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.
@damien2012eng the changes look really good! Other than the suggestions I made, I also have the following comments:
- If you aren't displaying some plots based on
has_single_example, then the description of that plot shouldn't be shown either or else it's confusing. - I see the following at the bottom of the waterfall plot: ``<Figure size 640x480 with 0 Axes>`
- We should mention the additional plots for the single examples in the RSMExplain tutorial and also which plots are available when.
tamarl08
left a comment
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 good! just had a couple of minor edits.
- Clearly show the plots that are unavailable instead of hiding them. - Fix the weird matplotlib message being shown for waterfall plot.
|
@tamarl08 , @damien2012eng I have made a bunch of changes.
Can you both please review the changes and also test the single-example and multiple-example runs to make sure that everything looks okay in the notebook? @damien2012eng can you also double check that when choosing the examples via |
|
Hi @desilinguist I have run experiments with both single and multiple responses, and the reports look good to me. I also provided multiple Thanks for the improvement! |
sample_idsas an option in the config filehas_single_exampleflag to turn on/off plots