ReliaSolve / cctbx_project

Computational Crystallography Toolbox
https://cctbx.github.io
Other
0 stars 0 forks source link

Fix FastOptimizer error turned up by testing and add regression test #258

Closed russell-taylor closed 1 year ago

russell-taylor commented 1 year ago

The ACTDup.pdb file has several ACT copies lined up such that they prefer to touch in the same orientation. A Carbon atom was brought in to switch the way they prefer to orient, switching them all backwards.

(done) Add this as a regression test inside the Optimizer.py function, pulling the contents of the file in as a string. Verify that the orientations for all of the Movers are as expected.

(done) The brute-force optimizer gets the correct result. The FastOptimizer gets a different one that has a lower energy. The clique optimizer also gets the right answer. Turning off score caching in FastOptimizer makes it work, so it is indeed a problem with the caching as opposed to the call order. The InteractionGraph built-in test completes successfully. Undoing the check to avoid re-moving already-correct Movers does not fix it; there was not a bug introduced when that was added. Printing shows that some of the cached scores are different from those that would be computed for that case; this happens for a bunch of different atom serial numbers. The atoms with differences have only one Mover in their state, but all Movers overlap with at least one other. Reverting the change the sped up _PairsOverlap did not fix the problem, so the bug was not introduced by that change. The _PairsOverlap function returns after the first overlapping pair is found, so will not detect all overlapping pairs. This sped up the calculation but won't completely fill out the atomMoverSets. Fixed this, which fixed the bug.

russell-taylor commented 1 year ago

@todo: See what improvement this makes on the regression runs.