Skip to content

Create setup, run, and analysis units for HybridTop Protocol#1773

Merged
IAlibay merged 102 commits intomainfrom
multi-unit-rfe
Jan 15, 2026
Merged

Create setup, run, and analysis units for HybridTop Protocol#1773
IAlibay merged 102 commits intomainfrom
multi-unit-rfe

Conversation

@IAlibay
Copy link
Member

@IAlibay IAlibay commented Jan 3, 2026

Bocked by #1771 and #1772
Fixes #1724

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

Developers certificate of origin

@IAlibay
Copy link
Member Author

IAlibay commented Jan 3, 2026

pre-commit.ci autofix

@IAlibay IAlibay marked this pull request as ready for review January 9, 2026 00:24
Copy link
Contributor

@hannahbaumann hannahbaumann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @IAlibay , this looks great! One thing I was wondering is whether you noticed any slowdowns having the three units instead of the one unit, e.g. due to serialization of things or recreation of the reporter?

from .hybridtop_protocols import RelativeHybridTopologyProtocol
from .hybridtop_units import RelativeHybridTopologyProtocolUnit
from .hybridtop_units import (
HybridTopologyMultiStateAnalysisUnit,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re unit names, do we already know how we would name the simulation and analysis units if we had a nonequilibrium protocol and would that match well with these names?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just going to call them HybridTopologyNEQUnit. BUT these names are not set in stone, we can redo them later.

# Get the relevant inputs
system = deserialize(setup_results.outputs["system"])
positions = to_openmm(np.load(setup_results.outputs["positions"]) * offunit.nm)
selection_indices = setup_results.outputs["selection_indices"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll somewhere have to document well which unit stores what to be easily findable for users.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally I would like to expose everything you would need in the analysis unit results - anything else we should add there?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, hadn't seen the selection_indices down there, those were mostly what I was thinking of!

@IAlibay
Copy link
Member Author

IAlibay commented Jan 9, 2026

any slowdowns

Not that I saw, the intermediate steps are rather quick so you shouldn't see much overhead.

IAlibay and others added 2 commits January 15, 2026 18:23
* fix test to have slightly better mocking
* add try/except nightmare

---------

Co-authored-by: IAlibay <IAlibay@users.noreply.github.com>
@IAlibay
Copy link
Member Author

IAlibay commented Jan 15, 2026

pre-commit.ci autofix

@IAlibay
Copy link
Member Author

IAlibay commented Jan 15, 2026

pre-commit.ci autofix

@github-actions
Copy link

No API break detected ✅

@codecov
Copy link

codecov bot commented Jan 15, 2026

Codecov Report

❌ Patch coverage is 81.51261% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.52%. Comparing base (7a897ad) to head (eba94fd).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
openfe/protocols/openmm_rfe/hybridtop_units.py 62.50% 30 Missing ⚠️
...tests/protocols/openmm_rfe/test_hybrid_top_slow.py 0.00% 18 Missing ⚠️
...s/protocols/openmm_rfe/test_hybrid_top_protocol.py 86.95% 15 Missing ⚠️
openfe/protocols/openmm_utils/serialization.py 93.10% 2 Missing ⚠️
...protocols/openmm_rfe/hybridtop_protocol_results.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1773      +/-   ##
==========================================
- Coverage   95.50%   92.52%   -2.98%     
==========================================
  Files         196      197       +1     
  Lines       16910    17046     +136     
==========================================
- Hits        16150    15772     -378     
- Misses        760     1274     +514     
Flag Coverage Δ
fast-tests 92.52% <81.51%> (?)
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.

@IAlibay IAlibay merged commit d3a5927 into main Jan 15, 2026
9 of 12 checks passed
@IAlibay IAlibay deleted the multi-unit-rfe branch January 15, 2026 21:35
atravitz added a commit that referenced this pull request Feb 5, 2026
* Turn Hybrid Topology protocol into 3 units.

---------

Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>
atravitz added a commit that referenced this pull request Feb 5, 2026
* update install instructions (#1744)

* update manifest to include all .gz files (#1746)

* update manifest to include all .gz files

* add min openmmforcefields pin

* bump cuda version to 11.8 (#1749)

* fix deprecation warning on view_components_3d (#1750)

* expose KartografAtomMapper alongside other AtomMappers (#1751)

* fix psutils import (#1779)

* fix psutils import

* swap import order

* [pre-commit.ci] pre-commit autoupdate (#1778)

updates:
- [github.com/tox-dev/pyproject-fmt: v2.8.0 → v2.11.1](tox-dev/pyproject-fmt@v2.8.0...v2.11.1)
- [github.com/astral-sh/ruff-pre-commit: v0.13.3 → v0.14.10](astral-sh/ruff-pre-commit@v0.13.3...v0.14.10)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Disable NAGL as a partial charge backend when OEToolkit is installed but not chosen as the registry backend. (#1762)

* NAGL can no longer be used with oechem installed if you are using the rdkit backend.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>

* rename openfecli fixture to avoid duplication with openfe fixture (#1755)

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

* openfe gather: add progress bar for loading JSONs (#1786)

* all the profiling

* make progress bar pretty

* revert typing thing

* news

* all the profiling

* make progress bar pretty

* revert typing thing

* news

* update expected outputs

* update expected outputs

* add dill mock to fix openfe docs build (#1792)

* Fix issue 1795 (#1796)

* Update method name in CLI YAML documentation

* Fix command option case in CLI YAML guide

* manually add absolute settings news item to changelog (#1821)

* manually add absolute settings news item to changelog

* add link

* Fixes docstring to clarify argument does not have a default (#1819)

* Fixes docstring to clarify argument does not have a default

* Modernize typing

* fix ligand network cropping bug (#1822)

* fix ligand network cropping bug

* move to upstream func

* whitespace

* fix pydantic deprecation of copy method

* fix pydantic deprecation of dict method - septop (#1831)

* Updated CHANGELOG for 1.8.1

* fix changelog links

* add openeye w/ python 3.13 test to CI (#1745)

* add openeye w/ python 3.13 test to CI

* only run openeye on unbuntu w/ python 3.13

* add explicit openeye no

* bump python version for conda cron (#1748)

* remove mypy rdkit pin (#1753)

* remove mypy rdkit pin

* try more explicit import

* Revert "try more explicit import"

This reverts commit 00cc2a3.

* disallow attr-defined error code

* try globally disabling attr-defined

* try specific override

* per-line ignores

* Make the multistate hybrid topology samplers not require the HybridTopologyFactory (#1768)

* make hybrid samplers not rely on htf but instead on the system & positions.

* Migrate validation to Protocol._validate in HybridTopologyProtocol (#1740)

* Migrate & add validation to Protocol._validate for hybrid topology Protocol

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hannah Baumann <43765638+hannahbaumann@users.noreply.github.com>
Co-authored-by: Josh Horton <Josh.Horton@newcastle.ac.uk>

* Move rfe protocol (#1769)

Move RFE protocol files around

* HybridTop Unit methods (#1770)

* Break down the Hybrid Top protocol unit into various methods.

---------

Co-authored-by: Hannah Baumann <43765638+hannahbaumann@users.noreply.github.com>

* Have two separate system generators for state A & B in hybrid protocol (#1772)

Make it so that the states A and B are generated using two different SystemGenerator objects, allowing for partial charge transformations.

* HybridTop structural analysis via API rather than CLI (#1771)

* change structural analysis from using CLI to using API

* making some language clearer on github-facing things (#1758)

* making some language clearer on github-facing things

* :

* add pre-commit link

* clean up

* Moving AFE Protocols around a bit (#1775)

* move the protocols results to a single file and deduplicate
* rename base units file
* move units out of method files
* move a few things in init

* Create setup, run, and analysis units for HybridTop Protocol (#1773)

* Turn Hybrid Topology protocol into 3 units.

---------

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

* update docs for multiple protocol units (#1793)

* Update docstring for hybridtop classes (#1794)

* Update docstring for classes

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

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

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Small improvement in RelativeHybridTopologyProtocol docstring (#1797)

Updated class docstring to clarify the use of Hybrid Topology scheme.

* turn off zenodo retry (#1802)

* turn off zenodo retry

* remove retry_if_failed arg

* only run CI on macos python 3.12 (#1803)

* Turn AFE protocols into multiple units (#1776)

* Split the AFE protocol units into setup, simulation, and analysis.

* move to src layout (#1805)

* Move openfe project layout to `src`.

* Temporarily build pooch from main w/ hotfix (#1806)

* build with pooch@main to see if hotfix works

* add link

* make CLI starting guide easier to find from the landing page (#1787)

* make CLI starting guide easier to find from the landing page

* Python API -> API Docs

* bump ci

* bump example notebooks pin to 2026.01.26

* change a word

* clarifying language

* Update pyproject.toml (#1813)

* Update pyproject.toml

* update classifiers

* refactor: clean up test data handling (#1815)

* import pooch_cache from conftest

* remove unused files from pooch cache

* move zenodo cache to conftest

* remove duplicate code

* min pin pooch to fix data fetching (#1820)

* fix ligand network cropping bug (#1822)

* fix ligand network cropping bug

* move to upstream func

* whitespace

* fix api break check path (#1825)

* fix pydantic deprecation of copy method (#1829)

* fix scope mismatch with zenodo data (#1828)

* fix scope mismatch with zenodo data

* fix session scope

---------

Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
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: Hannah Baumann <43765638+hannahbaumann@users.noreply.github.com>
Co-authored-by: Josh Horton <Josh.Horton@newcastle.ac.uk>
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.

Refactor base AFE unit into multiple units

3 participants