-
Notifications
You must be signed in to change notification settings - Fork 37
Fast InitContext #1125
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
Fast InitContext #1125
Conversation
| params::NamedTuple, fallback::Union{AbstractInitStrategy,Nothing}=InitFromPrior() | ||
| ) | ||
| return InitFromParams(to_varname_dict(params), fallback) | ||
| return new{typeof(params),typeof(fallback)}(params, fallback) |
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.
This code, which is present on main, is actually (sadly) quite bad code (written by yours truly). For the trivial model, it had the effect of making simple NamedTuples 10x slower because to_varname_dict returns a Dict{VarName,Any}, which is too loosely typed and leads to slow lookups etc.
Lines 851 to 855 in 08fffa2
| # Convert (x=1,) to Dict(@varname(x) => 1) | |
| function to_varname_dict(nt::NamedTuple) | |
| return Dict{VarName,Any}(VarName{k}() => v for (k, v) in pairs(nt)) | |
| end | |
| to_varname_dict(d::AbstractDict) = d |
Benchmark Report for Commit 699aa23Computer InformationBenchmark Results |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## py/fastldf #1125 +/- ##
==============================================
- Coverage 81.86% 81.72% -0.15%
==============================================
Files 41 41
Lines 3949 3956 +7
==============================================
Hits 3233 3233
- Misses 716 723 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c34ce8d to
7deaaab
Compare
|
DynamicPPL.jl documentation for PR #1125 is available at: |
699aa23 to
c35dff5
Compare
Much like how #1113 makes LogDensityFunction much faster by simply removing the use of Metadata, this PR makes InitContext much faster by also cutting out Metadata.
I mainly wanted to put this out there so that we have something to discuss on Monday's meeting.
Benchmarks
Here are some benchmarks on the trivial model. Obviously, this tiny model makes this PR look really good. But given that the speedups for NT and Dict are on the same scale as for Vector, I assume that it will generalise to other models in exactly the same way as discussed previously.
AD
On top of that, you can use this to run faster AD on NamedTuple inputs.
(This isn't new, you always could do it: this is just faster. Using
viinstead ofoavimakes Mooncake about 1.6x slower.)Note that this effectively provides a 'fast' implementation of NamedLogDensity (#880) although it should be mentioned that vector LogDensityFunction is still a lot faster for both Mooncake and Enzyme.
(ForwardDiff and ReverseDiff only work with vector-valued inputs.)
Outlook
I think the main thing that I learnt from this is that fast NamedTuple and Dict evaluation is already quite easily obtainable (this PR is < 10 lines) by composing
InitFromParamswithOnlyAccsVarInfo.This makes me think that what's now called
FastEvalVectorContextcan actually just be renamed into another flavour ofInitFromParams. That would, IMO, lead to an incredibly satisfying reduction of code complexity, and conceptually I find it extremely clear:So the combination of
InitFromParams+OnlyAccsVarInfogives you a clear-cut way of getting accs given params.This also means that generalising 'fast evaluation' to other types of parameters is incredibly easy: just implement a new method for
InitFromParamsand voila, it will straightaway work withOnlyAccsVarInfo.The only thing I'm not in love with is that
init!!starts to look a bit like a misnomer. Sure, I guess, it's still initialising accumulators for you. But I think this reinforces the notion I've had for a while now, thatInitFromParamswas actually about way more than just filling a VarInfo with specific params: the truth is thatInitFromParamsis actually a way to completely decouple parameter values from VarInfo.