Add CMAPTorsionForce support by jthorton · Pull Request #1695 · OpenFreeEnergy/openfe

jthorton · 2025-11-25T10:14:42Z

Fixes #1658 by adding support for CMAPTorsionForce not in the alchemical region for the relative hybrid topology protocol. Tests developed in jthorton/htf-tinker#2 are also included to add better coverage of the factory here.

Checklist

All new code is appropriately documented (user-facing code must have complete docstrings).
Added a news entry, or the changes are not user-facing.
Ran pre-commit by making a comment with pre-commit.ci autofix before requesting review.

Manual Tests: these are slow so don't need to be run every commit, only before merging and when relevant changes are made (generally at reviewer-discretion).

GPU integration tests
example notebook testing
packaging tests: run this for any large feature PRs or PRs that add test data.

Developers certificate of origin

I certify that this contribution is covered by the MIT License here and the Developer Certificate of Origin at https://developercertificate.org/.

jthorton · 2025-11-25T10:15:40Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

# Conflicts: # openfe/tests/conftest.py # openfe/tests/protocols/openmm_rfe/test_relative.py

jthorton · 2025-11-25T12:49:14Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

atravitz

with the addition of this test data, we should revisit our "what to zip" conversation.

openfe/protocols/openmm_rfe/_rfe_utils/relative.py

atravitz · 2025-11-26T19:01:16Z

openfe/protocols/openmm_rfe/_rfe_utils/relative.py

+            raise RuntimeError(f"Inconsistent CMAPTorsionForce between end states expected to be present in both"
+                               f"but found in old: {bool(cmap_old)} and new: {bool(cmap_new)}")
+
+        if cmap_new == cmap_old is None:


You check this both here and here, so this code path will likely not be hit by tests.

Reworked slightly to only check in the main _handle_cmap_torsion_force function.

IAlibay

Just seeing @atravitz's comment re: gzipping and I'm going to be a bit more opiniated.

openfe/tests/data/htf/t4_lysozyme_data/t4_lysozyme_solvated.pdb

IAlibay · 2025-11-26T20:38:59Z

openfe/protocols/openmm_rfe/_rfe_utils/relative.py


+    @staticmethod
+    def _verify_cmap_compatibility(
+            cmap_old: openmm.CMAPTorsionForce | None,


Could not account for the None? Also, indentation is odd here, did ruff formatting pass?

Somehow ruff did work :/, not sure what you mean with regards to the None.

Since you're testing for None before you get to this method, I was wondering if it made sense for you to not do that in both places. So technically if you wanted, you could just assume that these parameters are never None and that you did the checking ahead of time?

Alternatively, just don't check for None in the method that calls this method.

IAlibay · 2025-11-26T20:40:12Z

openfe/protocols/openmm_rfe/_rfe_utils/relative.py

+
+        # verify compatibility and extract numbers of maps and torsions
+        (
+            cmap_old,


It's unclear to me, why return the input cmaps?

Just so we don't have to pull them out twice, but happy to change this if it makes it easier to follow?

I'm confused, why would we be pulling them twice? I.e. aren't they just the same as what we give in the input?

Ohh you mean the force objects rather than the cmap grids, yeah we can stop returning the forces as well!

I think this is left over from the first design where the systems are passed in and these are extracted good catch!

openfe/protocols/openmm_rfe/_rfe_utils/relative.py

openfe/tests/helpers.py

openfe/tests/conftest.py

openfe/tests/protocols/openmm_rfe/test_relative.py

openfe/tests/conftest.py

codecov · 2025-11-27T00:11:55Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.15%. Comparing base (013b4b3) to head (1986599).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1695      +/-   ##
==========================================
- Coverage   95.41%   93.15%   -2.27%     
==========================================
  Files         185      187       +2     
  Lines       16045    16231     +186     
==========================================
- Hits        15310    15120     -190     
- Misses        735     1111     +376

Flag	Coverage Δ
fast-tests	`93.15% <100.00%> (?)`
slow-tests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>

jthorton · 2025-11-27T12:18:01Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

IAlibay

One question about that sigma_old change - my understanding is that it doesn't matter because the LJ component should be zeroed out with epsilon set to zero (and you have to keep sigma non zero because otherwise all the problems happen). But it's unclear to me if there's an ulterior motive behind the change.

news/cmap_support.rst

openfe/protocols/openmm_rfe/_rfe_utils/relative.py

openfe/tests/protocols/openmm_rfe/helpers.py

Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>

github-actions · 2025-12-01T11:39:58Z

No API break detected ✅

jthorton · 2025-12-01T11:51:43Z

One question about that sigma_old change - my understanding is that it doesn't matter because the LJ component should be zeroed out with epsilon set to zero (and you have to keep sigma non zero because otherwise all the problems happen). But it's unclear to me if there's an ulterior motive behind the change.

No thats correct, this is from the other general tests that I was doing on the HTF, I set it to what I expected it to be but as you point out this does not change the result as the energy is always zero with epsilon set to zero.

IAlibay

lgtm, thanks!

Adds support for CMAPTorsionForce in relative hybrid topology simulations. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com> Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>

add cmaptorsion support and htf tests

a0d5da6

[pre-commit.ci] auto fixes from pre-commit.com hooks

987e1e7

for more information, see https://pre-commit.ci

jthorton requested review from IAlibay and atravitz November 25, 2025 10:18

jthorton added 3 commits November 25, 2025 11:37

keep only cmap related tests

eda69d5

Merge remote-tracking branch 'origin/cmap_supp' into cmap_supp

f98e5bb

# Conflicts: # openfe/tests/conftest.py # openfe/tests/protocols/openmm_rfe/test_relative.py

fix mypy errors

9a58eeb

[pre-commit.ci] auto fixes from pre-commit.com hooks

cd35a40

for more information, see https://pre-commit.ci

atravitz reviewed Nov 26, 2025

View reviewed changes

IAlibay requested changes Nov 26, 2025

View reviewed changes

Merge branch 'main' into cmap_supp

c33c345

IAlibay reviewed Nov 26, 2025

View reviewed changes

openfe/tests/conftest.py Outdated Show resolved Hide resolved

fix issues with confusing units

a660b5e

Update openfe/protocols/openmm_rfe/_rfe_utils/relative.py

ded278f

Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>

jthorton mentioned this pull request Nov 27, 2025

Use serialized inputs to test the hybrid topology factory #1705

Open

PR feedback

501a11c

[pre-commit.ci] auto fixes from pre-commit.com hooks

fabda5d

for more information, see https://pre-commit.ci

jthorton mentioned this pull request Nov 27, 2025

Test the HTF with different combinations of nonbonded method #1706

Open

jthorton added 2 commits November 27, 2025 13:11

add news and test file

aa19425

fix test files

a715a72

IAlibay requested changes Nov 28, 2025

View reviewed changes

news/cmap_support.rst Outdated Show resolved Hide resolved

openfe/protocols/openmm_rfe/_rfe_utils/relative.py Outdated Show resolved Hide resolved

openfe/tests/protocols/openmm_rfe/helpers.py Show resolved Hide resolved

jthorton and others added 2 commits December 1, 2025 11:31

Update news/cmap_support.rst

c3e3436

Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>

move helpers, undo htf change

1986599

IAlibay approved these changes Dec 1, 2025

View reviewed changes

IAlibay merged commit 35a9999 into main Dec 1, 2025
13 checks passed

IAlibay deleted the cmap_supp branch December 1, 2025 14:19

atravitz mentioned this pull request Dec 4, 2025

release/1.8.0 #1734

Merged

7 tasks

Conversation

jthorton commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Developers certificate of origin

Uh oh!

jthorton commented Nov 25, 2025

Uh oh!

jthorton commented Nov 25, 2025

Uh oh!

atravitz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jthorton Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IAlibay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jthorton commented Nov 27, 2025

Uh oh!

IAlibay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 1, 2025

Uh oh!

jthorton commented Dec 1, 2025

Uh oh!

IAlibay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jthorton commented Nov 25, 2025 •

edited

Loading

jthorton Nov 27, 2025 •

edited

Loading

codecov bot commented Nov 27, 2025 •

edited

Loading