Skip to content
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

Do we need resume_from now that we have initial_state? #2171

Open
torfjelde opened this issue Feb 8, 2024 · 4 comments
Open

Do we need resume_from now that we have initial_state? #2171

torfjelde opened this issue Feb 8, 2024 · 4 comments

Comments

@torfjelde
Copy link
Member

AbstractMCMC@5 has initial_state as a mechanism to "resume" a chain.

This then begs the question: do we even need resume_from that is present in many sampler impls in Turing.jl?

resume_from=nothing,
initial_state=DynamicPPL.loadstate(resume_from),

AFAIK the only place resume_from is actually overloaded is for Chains:

https://github.com/TuringLang/DynamicPPL.jl/blob/ff68206d4b34e230c7e444e91fc60297dd5a5bd0/ext/DynamicPPLMCMCChainsExt.jl#L18-L27

IMO we should replace this with a simple getstate or something from Chains and then just use initial_state everywhere to stay consistent with the AbstractMCMC interface.

@torfjelde
Copy link
Member Author

torfjelde commented Feb 8, 2024

@devmotion @yebai thoughts?

@yebai
Copy link
Member

yebai commented Feb 14, 2024

I don't think we need resume_from anymore.

@devmotion
Copy link
Member

Yes, I think it's redundant and confusing, now that initial_state is an official AbstractMCMC keyword argument.

@torfjelde
Copy link
Member Author

I see #2115 did something on this, but it doesn't seem to be "complete", e.g.

Turing.jl/src/mcmc/hmc.jl

Lines 100 to 115 in c29d36e

if resume_from === nothing
# If `nadapts` is `-1`, then the user called a convenience
# constructor like `NUTS()` or `NUTS(0.65)`,
# and we should set a default for them.
if nadapts == -1
_nadapts = min(1000, N ÷ 2)
else
_nadapts = nadapts
end
# If `discard_initial` is `-1`, then users did not specify the keyword argument.
if discard_initial == -1
_discard_initial = discard_adapt ? _nadapts : 0
else
_discard_initial = discard_initial
end

is hit when resume_from === nothing which will be the case even if the user specifies initial_state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants