Repository navigation
MOLINFO WHOLE: check that the reference is not split by the periodic boundary (#1492) - #1494
aalhossary wants to merge 2 commits into
Conversation
…boundary (plumed#1492) With MOLINFO ... WHOLE, WHOLEMOLECULES follows a spanning tree built on the reference coordinates, and the reference is taken as whole on trust. If part of it is wrapped across the periodic boundary, the tree links that part to the wrong atoms through a link that is already its own minimum image, and WHOLEMOLECULES then splits whole molecules in every frame, silently. When the reference pdb has a CRYST1 record that can be the box it was written in (space group P 1, not the 1 A placeholder, not degenerate), Tree now builds the tree a second time on minimum-image distances and walks it over the reference. Atoms that come back moved by a box vector are on the wrong side, and PLUMED stops with an error that names them. A linear-time grid search skips the second tree when no atom comes close to a periodic image of another, which is the usual case for a whole reference.
…cells (plumed#1492) rt-molinfo-whole-check reads a whole and a split copy of trp-cage (1L2Y, model 1) with MOLINFO WHOLE and reports which references are rejected and which atoms are named; copies without a usable CRYST1 record are accepted. rt-molinfo-whole-repair makes five split frames whole (rhombic dodecahedron, truncated octahedron, triclinic and cubic cells; atom- and residue-wise wrapping) with a whole reference.
|
Thanks @aalhossary ! I am not sure this is a good idea. I see that the test is only done when using |
|
Thanks @GiovanniBussi. I agree that making the reference whole is the user’s job, and the PR does not try to do it, nor to define one way of doing it. It only checks the reference against the box written in the same file, and only when there is one (CRYST1, space group P 1). BTW, An ambiguous case does exist: a very sparse group (e.g. every 10th Cα) in a tight box can have its own periodic images closer than its longest tree link. There, the check may reject a whole reference, but If you prefer, I can make it a warning in the log instead of an error, or put it behind a keyword. Which would you rather have? |
|
The full test report for this PR, for reference: how the check and the repair were tested against ground truth. Summary. 89,780 synthetic references with known truth (117,710 runs, one set repeated with the fast path). In the 88,016 cases where the geometry is unambiguous (see D ≤ L below), every split reference was rejected, every whole one accepted, the displaced atoms named exactly, and the frame repaired correctly. The repair agrees with Synthetic references. Each case takes a whole structure, places it in a cell, wraps it, and runs PLUMED twice: (A) the result as the
2,186 of the all-atom cases had only 1–3 atoms across the boundary; all were caught. Without the check, The ambiguous case, D ≤ L. L is the longest link of the group's nearest-neighbour tree; D is the shortest distance between an atom and a periodic image of another atom of the same whole group. When D > L, the tree in the box equals the plain tree, so the check is exact. When D ≤ L, an image is as close as a real neighbour, and "whole" is not defined by the coordinates alone. This happened only for every 5th / 10th Cα in tight boxes: 1,666 of the 45,570 selection cases, 1,568 of them answered wrongly. Outside tools and real files.
The 13 real-file cases where the verdict differs from "is the molecule whole":
The relabelled cells are why only P 1 (or a blank space group) is checked. The split GB3 reference of #1492 is rejected with its 12 atoms named, also with its cell 2% larger or smaller (an NPT frame) and with a blank space group. Cost (optimized serial build, best of 3):
Regtests. Full local suite (debug build, clang/libc++): 817 run; the one failure, |
Description
Closes #1492.
MOLINFO ... WHOLEis taken on trust. If the reference pdb is in fact split by the periodic boundary,WHOLEMOLECULESsplits whole molecules in every frame, silently (details and a reproducer in #1492). This PR turns that silent wrong result into an error at setup:Tree::buildTree()(used byWHOLEMOLECULESwithMOLINFO ... WHOLE) now callscheckReferenceIsWhole()for its group of atoms. When the reference has a usable box, the check builds the tree a second time on minimum-image distances in that box and walks it over the reference: a whole reference comes back unchanged, wrapped atoms come back moved by a box vector, and those are the atoms named in the error. The tree itself is unchanged.A usable box (
GenericMolInfo::getReferenceBox()) is a CRYST1 record with space group P 1, which is what simulation programs write. No CRYST1, a degenerate cell, the 1 Å placeholder cube, or a crystallographic cell (any other space group) skip the check, so those references behave exactly as before.PDBnow keeps the space group for this.Speed: a linear-time grid search (
hasCloseImage()) first checks whether any atom comes closer to a periodic image of another atom than the longest link of the tree. If none does, the second tree is provably the same as the first, and it is skipped. This is the usual case for a whole reference. Extra setup time (optimized build, best of 3, rhombic dodecahedron / triclinic / cubic cells):driverrun without the checkThe check runs per group, not on the whole MOLINFO file: on a solvated reference, the solvent always spans the box, so a file-wide check would flag it.
Target release
I would like my code to appear in release master (v2.11)
Type of contribution
Copyright
Tests
New regtests, on trp-cage (1L2Y, model 1):
basic/rt-molinfo-whole-checkreads a whole and a split copy withMOLINFO ... WHOLEand reports which are rejected and which atoms are named. Copies without a usable CRYST1 (none, placeholder, zero, crystallographic space group) are accepted. Without the check, this test fails.basic/rt-molinfo-whole-repairmakes five split frames whole with a whole reference (rhombic dodecahedron, truncated octahedron, triclinic and cubic cells; atom- and residue-wise wrapping).Beyond the regtests, the check and the repair were tested against ground truth outside PLUMED:
WHOLEMOLECULESis wrong in more than half of the cases even with a whole reference, and the error message names this cause too.gmx trjconv -pbc whole(bond graph from the tpr) on 6 systems × 60 split frames: identical to PLUMED's repair in all 360.--debug-ddon 2 and 4 ranks and--multi 2: output byte-identical to serial; a split reference raises the error on every rank, without hanging.basic/rt-make-fortran08, which fails the same way without this change in my clang/libc++ build. All 37 regtests that useWHOLEorWHOLEMOLECULESpass.Prepared with the help of Claude (Anthropic).