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

Error when using MCMCEnsemble on models containing Module types #89

Open
cpfiffer opened this issue Dec 9, 2021 · 4 comments
Open

Error when using MCMCEnsemble on models containing Module types #89

cpfiffer opened this issue Dec 9, 2021 · 4 comments

Comments

@cpfiffer
Copy link
Member

cpfiffer commented Dec 9, 2021

Originally detected in HighDimensionalEconLab/HMCExamples.jl#28.

Essentially, we generate a Module and use that as a pseudo-function struct, but its inclusion in the model means that the model deepcopy

models = [deepcopy(model) for _ in interval]

fails as Module cannot be deepcopied. Unfortunately it's a little tricky to detect this kind of weird case in advance. Perhaps it would be nice to add keywords disabling any of the deepcopy calls for users who don't mind so much?

@cpfiffer
Copy link
Member Author

cpfiffer commented Dec 9, 2021

This is kind of a RFC, by the way -- I'm not sure if there's a better way than a keyword argument.

@devmotion
Copy link
Member

Essentially, we generate a Module and use that as a pseudo-function struct

This seems a very surprising and non-standard use of Modules 👀

Maybe we do something similar to what we do in SciML (https://diffeq.sciml.ai/stable/features/ensemble/) and support to specify if a safety deepcopy of the model and sampler are called and what reduction function should be used.

@wupeifan
Copy link

wupeifan commented Dec 10, 2021

Our current solution is to override the deepcopy of modules by passing the references only, which is fine for now (i.e., overriding this line https://github.com/JuliaLang/julia/blob/master/base/deepcopy.jl#L33); whether deepcopying a module is a good move or not is up to you guys, but this issue (I think) is temporarily resolved. (See HighDimensionalEconLab/DifferentiableStateSpaceModels.jl#70)

@cpfiffer
Copy link
Member Author

I think we might as well implement the safecopy stuff just because it is good practice, but it is no longer a priority.

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