-
Notifications
You must be signed in to change notification settings - Fork 47
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 test.sample.test_sample.test_pipeline #1208
Conversation
There is really no need to run `pypesto.visualize.sampling.sampling_1d_marginals` over 20 times. It seems this was the cause of ICB-DCM#1077. In some cases, plotting takes over 10min (cancelled at some point, not sure it would ever complete). Maybe somebody using this function would like to investigate that further...
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1208 +/- ##
========================================
Coverage 84.08% 84.09%
========================================
Files 148 148
Lines 11775 11775
========================================
+ Hits 9901 9902 +1
+ Misses 1874 1873 -1 ☔ 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.
Agreed, I don't see a need for any testing of visualization here, it's tested elsewhere.
904.16s call test/sample/test_sample.py::test_pipeline[Emcee-gaussian]
I'm surprised this took so long. It should be plotting just 1000 (n_walkers * n_samples
) samples in total, for a single parameter...
Ditto. Might be good to dig deeper. The two times I was able to reproduce long run times were both with emcee. The second time was "only" 210.49s, though. Maybe seaborn / mpl is doing something funny there, not sure. |
Looks like it, I can plot a histogram with matplotlib almost immediately, but seaborn takes a lot of time, when using a vector of parameter samples from Regarding the variability, this seems to be dependent on the number of burn-in (Geweke test) samples removed from the visualization. |
There is really no need to run
pypesto.visualize.sampling.sampling_1d_marginals
over 20 times.It seems this was the cause of #1077. In some cases, plotting takes over 10min (cancelled at some point, not sure it would ever complete). Maybe somebody using this function would like to investigate that further...