-
Notifications
You must be signed in to change notification settings - Fork 37
Reduce allocations for VarInfo #1115
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
base: main
Are you sure you want to change the base?
Conversation
Benchmark Report for Commit 6e633d2Computer InformationBenchmark Results |
|
DynamicPPL.jl documentation for PR #1115 is available at: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1115 +/- ##
===========================================
- Coverage 81.23% 37.48% -43.75%
===========================================
Files 40 40
Lines 3805 3836 +31
===========================================
- Hits 3091 1438 -1653
- Misses 714 2398 +1684 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The only test failure so far (not done though) is an I don't immediately see why this should be deep problem that couldn't be solved with some function boundaries. @torfjelde, do you have any wise words of warning about there being dragons here? |
|
One potential danger of using views in julia> using DynamicPPL, Distributions
julia> @model f() = x ~ Normal(); vi = VarInfo(f()); xs = [0.0]
1-element Vector{Float64}:
0.0
julia> vi = DynamicPPL.unflatten(vi, xs);
julia> vi[@varname(x)] = 1.0 # alternatively: vi = last(DynamicPPL.init!!(f(), vi, InitFromParams((; x = 1.0))))
1.0
julia> xs
1-element Vector{Float64}:
1.0I'm fairly sure the aliasing doesn't matter as long as Currently less sure about what happens if On the other hand I think that using the view in |
Inspired by #1113, and together with @penelopeysm, we tried to squeeze down allocations caused in various parts of VarInfo. This does it to a point where evaluating the LogDensityFunction of a very trivial model causes no allocations what so ever:
The same thing on
mainis:This is not necessarily meant to be merged. We have not thought carefully at all about the effect of
@view. This shows that it can be done, and what the important parts are.