-
Notifications
You must be signed in to change notification settings - Fork 36
Make DynamicPPL.TestUtils.run_ad
return both the primal and gradient benchmarks
#1009
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
Benchmark Report for Commit 2c4d913Computer Information
Benchmark Results
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1009 +/- ##
=======================================
Coverage ? 82.26%
=======================================
Files ? 38
Lines ? 3947
Branches ? 0
=======================================
Hits ? 3247
Misses ? 700
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@penelopeysm sorry for missing this! is it a good time to review it? |
@sunxd3 Sure, now is as good a time as any other :) |
there are quite a lot of code changes now, should we do a rebase of this branch to something? sorry for asking |
Oh, dear, sorry. I'll sort that. |
d7276d7
to
2c4d913
Compare
DynamicPPL.jl documentation for PR #1009 is available at: |
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 great! ready to go from my side once CI passes
Pull Request Test Coverage Report for Build 16876123332Details
💛 - Coveralls |
Thanks @sunxd3! |
run_ad(...; benchmark=true)
used to benchmarklogdensity
andlogdensity_and_gradient
, then take the ratio and return that.This PR makes it return both benchmark results because, I mean, why not? We already have this information.
There is a bit of dangerous scope creep here in that this function is turning into a replacement for TuringBenchmarking. (Note it can't benchmark the primal alone without a gradient though so it's still not the same.)
But I think it's still useful to know both figures separately. For example, in the CI benchmarks table, we tabulate both times.
Unfortunately I attempted to change the CI benchmark setup to use the new functionality #1002 and promptly got bitten by the bug in #1001. So this PR doesn't change that, not for now, at least.