Add fixed atoms feature to ML-FSM

[joegomes]

This pull request adds:

• Option to fix atom coordinates during interpolation and optimization

In order to keep atom coordinates fixed during interpolation and optimization steps, several changes are needed:

1. When two molecules are aligned, the rotation vector should be calculated based on aligning the fix atoms. If atoms in the reactant and product structures were fixed during optimization, this should perfectly align those positions. The resulting rotation matrix is then applied to the entire molecule. This might break when there are less than 3 fixed atoms and should be addressed somehow before merging.
2. When interpolation coordinates are constructed, all redundant and Cartesian coordinates involving fixed atoms are removed either directly or before construction by altering the connectivity matrices. The corresponding columns in the B-matrix are then zero and we can guarantee that atom positions are unchanged during coordinate backtransform. No post-corrections are applied in the LST interpolation case, but fixed atoms were observed correctly in a test of the method.
3. During optimization, the gradient on fixed atoms should be zeroed to prevent coordinate updates. Currently only CartesianOptimizer is supported.
4. During the string alignment step when writing the FSM string to disk, we should use the "fixed" variant of the alignment described above.

I'm guessing there are code tests that will fail, but I want to address the final point of (1) before fixing those issues. To test this out, I've added a job for proton transfer TS on a zeolite cluster where the terminal hydrogens are fixed to maintain crystallographic position. The job may be ran like python fsm_example.py data/09_proton --calculator xtb --stepsize 0.2 --fixed "1-12". Here, atoms 1-12 are fixed. The code for parsing the fixed atoms flag should be flexible to handle mixed strings of comma-separated and hyphenated range values.

[codecov]

Codecov Report

❌ Patch coverage is 38.80597% with 41 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/mlfsm/utils.py	22.72%	17 Missing ⚠️
src/mlfsm/geom.py	12.50%	14 Missing ⚠️
src/mlfsm/coords.py	61.53%	5 Missing ⚠️
src/mlfsm/cos.py	62.50%	3 Missing ⚠️
src/mlfsm/opt.py	75.00%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[jonmarks12]

Hi @joegomes, I've added a 2 commits that satisfy some of the type checking and linting. The only meaningful changes I have added are in the fsm_example.py where I removed a duplicate parse_indices function, and in coords.py where I shifted some code around such that fixed_atoms is an attribute of the base coordinates class so both redundant and Cartesian simply inherit it. Please look over it and make sure it still behaves identically to your original code, open to suggestions about how to better organize all of this. If all looks good feel free to merge into main, I'll write a fixed atoms test case before we cut a new version to appease codecov bot

[joegomes]

Hi @jonmarks12, I've tested these changes and things look good on my end! I forgot to update my code for Cartesian coordinates from my previous fixed atoms implementation, so LST interpolation was temporarily broken. I've fixed it now, so I'm good to merge this in.

[jonmarks12]

I believe this results in identical behavior to the code added in my prior commit on this pr. My original intent was to standardize fixed_atoms as an attribute defined once in the base Coordinates class, which is what the Redundant class currently uses.

As far as I can tell this doesn't introduce any functional difference, but it does create a stylistic difference between cartesian and redundant object implementations. If you agree, I'd suggest rolling this back so both classes follow the same pattern, and future classes can avoid having these ~5 lines every time.

if there is a difference I'm missing here please let me know, happy to keep it if so. Thanks!

[joegomes]

Thanks for catching this. I originally had some failing test cases with LST and CART interpolation but I can't reproduce those errors now after rolling back the changes to Cartesian. I've tested this updated branch with and without fixed atoms with the three interpolation methods (all fixed step size) and it's all working locally.