-
Notifications
You must be signed in to change notification settings - Fork 35
Improvements to DynamicPPLBenchmarks #346
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
… for downstream tasks
This might be helpful for running benchmarks via CI - https://github.com/tkf/BenchmarkCI.jl |
@torfjelde should we improve this PR by incorporating Also, https://github.com/TuringLang/TuringExamples contains some very old benchmarking code. |
Pull Request Test Coverage Report for Build 13810813889Details
💛 - Coveralls |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main TuringLang/DynamicPPL.jl#346 +/- ##
=======================================
Coverage 84.56% 84.56%
=======================================
Files 34 34
Lines 3830 3830
=======================================
Hits 3239 3239
Misses 591 591 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I am not aware of wide use of It is okay to duplicate / re-implement the |
Thanks @shravanngoswamii, looks good, nice touch on making the commit hash a link too. I pushed a few commits that
Anyone object to merging this? |
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 @mhauru @shravanngoswamii and @torfjelde. Great that this is finally ready to merge (it only takes a few years)!
I'd like to review, if that's ok |
I hope nothing too onerous, and I promise these are my last comments |
Co-authored-by: Penelope Yong <penelopeysm@gmail.com>
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.
LGTM! Well done to everyone on this nice piece of work.
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.
@shravanngoswamii, do you want to check that you're happy as well, and hit merge if so?
@mhauru, Everything looks good to me! Can you hit the merge button, I am not authorized to push to main branch in this repo! |
…to tor/benchmark-update
Finally, It's merged! Thank you, @yebai @mhauru @penelopeysm and @torfjelde! |
Thanks @shravanngoswamii, good work! |
Produces results such as can be seen here.