Deepcopy adaptor before starting sampling #382
Conversation
|
I have this terrible fear that somebody out there might have a workflow which relies on the adaptor being mutated in a predictable manner (https://xkcd.com/1172/) .... Of course, it's in the tests 😄 AdvancedHMC.jl/test/adaptation.jl Lines 16 to 18 in 5d56902 |
penelopeysm
force-pushed
the
py/random
branch
2 times, most recently
from
39a4c24 to
9998e10
Compare
This avoids the unintuitive behaviour seen in #379
penelopeysm
force-pushed
the
py/random
branch
from
9998e10 to
9015dd4
Compare
|
Thanks @penelopeysm !
Despite opening #379 (comment) I'm actually I inspect the adaptor sometimes after sampling in order to see the mass matrix it adapts. |
| Base.isnan(v::DualValue) = any(isnan, v.value) || any(isnan, v.gradient) | ||
| Base.isnan(v::AbstractVecOrMat) = any(isnan, v) | ||
| Base.isnan(z::PhasePoint) = isnan(z.ℓπ) || isnan(z.ℓκ) |
There was a problem hiding this comment.
Indeed, that's a hangover from #381 because I was a bit lazy to fix the commit history
There was a problem hiding this comment.
Realised this one has a double bump in Project.toml too. - let's decide on that PR first then I'll modify accordingly
The implementation of
sample()in AdvancedHMC mutates the adaptor passed in, which is both unintuitive (it'ssamplenotsample!) and also undocumented (see #379).This PR:
sample()tosample_mutating_adaptor()(other suggestions for names welcome – I didn't want to usesample!as that has another meaning)sample()which performs a deepcopy of the adaptor before handing over tosample_mutating_adaptor()Random.GLOBAL_RNGtoRandom.default_rng(), which I think is the proper modern method of accessing the global PRNG state.Draft for now to check that CI passes + also to wait for #381 to be merged. (or rejected 😱 )