Support pickling of calculators with Ctilde basis sets#1
Merged
Conversation
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
pmrv
commented
Jul 15, 2026
pmrv
left a comment
pmrv
left a comment
Owner
Author
There was a problem hiding this comment.
also test .yace file from the tests direction
run a minimization of the dimer to test a few more forces/energies for consistency.
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
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
Description of Changes
Extends the pickle support from the
picklebranch (ICAMS#99) to calculators built from Ctilde potentials, not just B-basis ones.PyACECalculator.__reduce__previously always calledself.basis.to_BBasisConfiguration(), which only exists onACEBBasisSet, so pickling a calculator loaded from an.ace/.yacepotential failed. The reducer now branches on the basis type: B-basis potentials are still dumped as a fullBBasisConfiguration(portable across machines and directories), whileACECTildeBasisSetobjects are passed through directly, relying on the native pickle support already present in their pybind11 bindings. Ctilde basis sets cannot be converted back to a B-basis representation, so a pass-through is the only option there.recursive_evaluator,recursive,gamma_lower_bound, etc.) are now preserved across the round trip via a module-level_unpickle_calculatorhelper, so e.g. a recursive Ctilde evaluator is restored as such instead of falling back to the defaults.todict()state from the reduce tuple, which previously leaked the originalbasis_setparameter (e.g. a file name) into the unpickled instance's__dict__(thepopresult was discarded).Checklist
License Agreement
By submitting this pull request, I agree that:
🤖 Generated with Claude Code
https://claude.ai/code/session_01BHj67uGNHN5bMH7LPoCR1J
Generated by Claude Code