Make PyACECalculator deal with pickle#99
Open
pmrv wants to merge 4 commits into
Open
Conversation
The default reducer puts out the evaluator and a bunch of other C++ extension types that pickle then chokes on. Side step the problem here by providing a reducer that just makes unpickling equivalent to a new instantiation. The only difficulty here is that basis_set stays a file name if initialized like that and that PyACECalculator.__init__ expects it as a positional argument. Avoid the filename by dumping the whole bbasis config, so that unpickling can happen on other machines or in different locations.
ACECTildeBasisSet cannot be converted back to a BBasisConfiguration, so the previous reducer failed for calculators loaded from .ace/.yace potentials. Pass the Ctilde basis set through as is instead; its pybind11 binding already provides native pickle support. While at it, preserve the constructor kwargs (recursive_evaluator, gamma bounds, etc.) across the pickle round trip via a module-level helper, and drop the stale todict() state that previously leaked the original basis_set parameter (e.g. a file name) into the unpickled instance. Add round-trip tests for B-basis, Ctilde and recursive Ctilde calculators. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHj67uGNHN5bMH7LPoCR1J
3 tasks
Address review: cover the Al-Ni .yace test potential and compare energies/forces along a whole BFGS dimer minimization trajectory between the original and the unpickled calculator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHj67uGNHN5bMH7LPoCR1J
Support pickling of calculators with Ctilde basis sets
Author
|
.yace based potentials are also supported now, and there some tests also that pass locally. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Template
Thank you for contributing to our project! Please review the checklist and fill out the details below.
Description of Changes
The default reducer puts out the evaluator and a bunch of other C++ extension types that pickle then chokes on. Side step the problem here by providing a reducer that just makes unpickling equivalent to a new instantiation. The only difficulty here is that basis_set stays a file name if initialized like that and that PyACECalculator.init expects it as a positional argument.
Avoid the filename by dumping the whole bbasis config, so that unpickling can happen on other machines or in different locations.
Checklist
License Agreement
By submitting this pull request, I agree that:
Thank you for your contribution!