-
Notifications
You must be signed in to change notification settings - Fork 220
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
Speed up notebook run action #1147
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1147 +/- ##
=======================================
Coverage 95.61% 95.61%
=======================================
Files 39 39
Lines 4063 4063
=======================================
Hits 3885 3885
Misses 178 178 ☔ View full report in Codecov by Sentry. |
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.
Thanks @wd60622 ! I left some small questions and comments! Looks cool!
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
I am looking at some of the logs. Seems like it was taken down from 40 minutes to 10 minutes 😆 |
@wd60622 This is a new trick for me, hehe. How does the injected code "replace" the sampling step in the notebook? Can you maybe push an error in a notebook and see if it get caught (we can then revert the commit). This seems like magic XD |
The injected first cell will run the "injected.py". This is before any imports which makes python think that pm.sample is mock_sample and pm.Flat = pm.Normal and pm.HalfFlat = pm.HalfFlat because they are cached even when they are imported from our modules |
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.
ok! I thnik I get it now :D Very nice! After removing the commented lines (see comment above), feel free to merge 🚀
Closes #1141
This runs into some issues with sampling Flat distribution, loading the model, and some sample_stats
This potentially could reduce the run time of the notebooks to 1/3 of the original time. One local notebook run when from 2.5 minutes to 19 seconds.
CC: @juanitorduz