Repository navigation
Do not serialize ExtraPoint children on atoms - #1439
Merged
Merged
Conversation
Atom.__getstate__ carried the children list, so deepcopy(atom) cloned each child ExtraPoint into an orphan object that was never added to the new structure's atom list. Structure.copy() rebuilds bonds afterwards, appending the real copied ExtraPoint to the same children list, which left every parent atom with a stale ExtraPoint partner (idx == -1) and broke rediscover_molecules during GROMACS-to-Amber conversion. children is derived state populated only by bond_to, like the partner lists that are already excluded from __getstate__. Drop it so copied atoms start empty and are repopulated by bond rebuilding. Fixes ParmEd#1407
swails
approved these changes
Oct 9, 2026
Contributor
|
Thanks! |
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
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.
Fixes #1407.
Atom.__getstate__included thechildrenlist, sodeepcopy(atom)(whichStructure.copycalls per atom) cloned each childExtraPointinto an orphan object. Those orphans were never added to the new structure's atom list, but the real copiedExtraPointwas appended to the samechildrenlist whenStructure.copyrebuilt theBondobjects. Every atom bonded to an extra-point parent ended up with a staleExtraPointpartner whoseidxstayed -1, which maderediscover_moleculessplit TIP4P waters and raisedMoleculeError.childrenis derived state. It is populated only bybond_toand rebuilt whenever bonds are rebuilt, so this drops it from__getstate__to make copied or unpickled atoms start with an empty list, matching the_bond_partnerslists that are already excluded and repopulated the same way.Atom.__setstate__initializeschildrenbefore applying the stored dict, so old pickles that still carry the key restore unchanged.Verification:
python -m pytest test/test_parmed_amber.py test/test_parmed_topologyobjects.py test/test_parmed_structure.py test/test_parmed_formats.pygives 281 passed and 17 skipped. The pre-existingtest_optimized_readerfailure is an unbuilt_rdparmextension on this machine and fails on master too. The new test fails on unfixed code.