Skip to content

Speed up HTF creation#1858

Open
jthorton wants to merge 7 commits intomainfrom
htf_speed
Open

Speed up HTF creation#1858
jthorton wants to merge 7 commits intomainfrom
htf_speed

Conversation

@jthorton
Copy link
Collaborator

@jthorton jthorton commented Feb 25, 2026

Try to speed up the HTF creation, targeting the torsion force and nonbonded exceptions as they scale terribly with system size. Local testing on a small protein-ligand system shows good improvements

Main

systems built starting test
Handled harmonic bonds in 0.36160308588296175 seconds
Handled harmonic angles in 1.0089957918971777 seconds
Handled periodic torsions in 7.016903054900467 seconds
Handled nonbonded interactions in 25.400564597919583 seconds
HybridTopologyFactory took 37.79149968735874 seconds

This branch

systems built starting test
Handled harmonic bonds in 0.3190057808533311 seconds
Handled harmonic angles in 0.9470380647107959 seconds
Handled periodic torsions in 0.24904045276343822 seconds
Time to handle original exceptions 0.51 seconds
Handled nonbonded interactions in 2.244331883266568 seconds
Handled nonbonded exceptions in 0.04826593864709139 seconds
HybridTopologyFactory took 7.013191535137594 seconds

We probably need the tests in #1856 to confirm that nothing is broken with these changes.

TODO

  • test on membrane system

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: you can run pre-commit locally or comment on this PR with pre-commit.ci autofix.

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).

Developers certificate of origin

@jthorton
Copy link
Collaborator Author

pre-commit.ci autofix

@mikemhenry
Copy link
Contributor

Oh this is very promising -- how stable are these timings? Do you think it would be worth making a test that ensures (for this test case) HybridTopologyFactory takes less than 10 seconds to build (might need to adjust depending on how long it takes on CI) but it would be nice to have some checks we don't lose these speed-ups later?

@jthorton
Copy link
Collaborator Author

how stable are these timings?

Locally, they are within a 3 second window so we could make a timing test, though I am not sure how common that is?We could also provide a timing script to be run whenever someone touches the HTF, which should not be often. Any preference @atravitz ?

@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

❌ Patch coverage is 92.53731% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.35%. Comparing base (a64bd44) to head (bccc53c).

Files with missing lines Patch % Lines
...openfe/protocols/openmm_rfe/_rfe_utils/relative.py 92.53% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1858      +/-   ##
==========================================
- Coverage   95.00%   92.35%   -2.65%     
==========================================
  Files         204      204              
  Lines       17640    17618      -22     
==========================================
- Hits        16758    16271     -487     
- Misses        882     1347     +465     
Flag Coverage Δ
fast-tests 92.35% <92.53%> (?)
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.

@github-actions
Copy link

No API break detected ✅

@jthorton
Copy link
Collaborator Author

With bccc53c the timings are now

systems built starting test
Handled harmonic bonds in 0.04707380570471287 seconds
Handled harmonic angles in 0.1307398546487093 seconds
Handled periodic torsions in 0.2686290815472603 seconds
Handled nonbonded interactions in 2.285492484457791 seconds
Handled nonbonded exceptions in 0.04942901339381933 seconds
HybridTopologyFactory took 6.055246475152671 seconds

@mikemhenry
Copy link
Contributor

There are a few ways to track regressions, one cool way is to make sure the algo doesn't change the way it scales: https://pythonspeed.com/articles/big-o-tests/ but we don't really have things set up to make that easy (but it would be good to know how the HTF scales with system size, so it might be worth doing)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants