BenchmarkToolsExt #861
BenchmarkToolsExt #861
Conversation
penelopeysm
force-pushed
the
py/benchmarks
branch
from
827217b to
e9966ff
Compare
There was a problem hiding this comment.
Generally, most of the code has just been shifted around, without any real modifications (the only new things are those needed to make it work, e.g. having the function itself be declared in src/DynamicPPL.jl so that it can be extended in the extension). Just a couple of clarifying comments.
| make_benchmark_suite( | ||
| [rng::Random.AbstractRNG,] | ||
| model::Model, | ||
| varinfo_choice::Symbol, | ||
| adtype::ADTypes.AbstractADType, | ||
| islinked::Bool |
There was a problem hiding this comment.
This function previously used to take a symbol as the adtype argument (e.g. :forwarddiff) and then use an internal function to convert it to an ADType. I think just passing the ADType itself is better because the ADTypes package is widely understood, more flexible, and doesn't require people to extend the symbol -> adtype function if they want to use a custom ADType, or figure out which symbol they need to use.
| data_1k = randn(rng, 1_000) | ||
| data_1k = randn(StableRNG(23), 1_000) | ||
| loop = Models.loop_univariate(length(data_1k)) | (; o=data_1k) | ||
| multi = Models.multivariate(length(data_1k)) | (; o=data_1k) | ||
| loop, multi | ||
| end | ||
| loop_univariate10k, multivariate10k = begin | ||
| data_10k = randn(rng, 10_000) | ||
| data_10k = randn(StableRNG(23), 10_000) |
There was a problem hiding this comment.
There are a bunch of changes to rng here. The point of using a fresh RNG object for every sampling call is to make sure that the values sampled don't change when we add or remove models.
We discussed this previously on Turing's test suite, cf. TuringLang/Turing.jl#2433 (comment)
Benchmark Report for Commit e9966ffComputer InformationBenchmark Results |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## breaking TuringLang/DynamicPPL.jl#861 +/- ##
============================================
- Coverage 84.87% 84.30% -0.58%
============================================
Files 34 35 +1
Lines 3815 3841 +26
============================================
Hits 3238 3238
- Misses 577 603 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Thanks, @penelopeysm. I looked at this and now feel the function make_benchmark_suite doesn't add much beyond what we have. In particular, if we make improvements to LogDensityFunction suggested in #863 and #862, then we no longer need the helper function make_benchmark_suite.
|
Yeah, that's a fair comment. I kind of wrote the same in the docs, it's easy enough to just benchmark logdensity_with_gradient. |
This moves the (reusable bits of the) code from the DynamicPPL benchmarks into a BenchmarkTools extension, so that other people can use it.
It also adds docs and changelog entries. Docs preview at https://turinglang.org/DynamicPPL.jl/previews/PR861/api/#Benchmarking-Utilities
I thought tests might be a bit extreme, I don't see how to test this. The CI benchmarking job does effectively test that it works.
Closes TuringLang/TuringBenchmarking.jl#43