Add structural analysis to the SepTop protocol by hannahbaumann · Pull Request #1982 · OpenFreeEnergy/openfe

hannahbaumann · 2026-05-28T10:05:26Z

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

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

hannahbaumann · 2026-05-28T10:06:06Z

pre-commit.ci autofix

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

hannahbaumann · 2026-05-29T11:04:42Z

This is what it would look like for the complex right now.

codecov · 2026-05-29T13:30:30Z

Codecov Report

❌ Patch coverage is 96.98276% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.62%. Comparing base (5347082) to head (a992d84).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/openfe/protocols/openmm_septop/base_units.py	93.06%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1982      +/-   ##
==========================================
- Coverage   94.94%   90.62%   -4.32%     
==========================================
  Files         216      217       +1     
  Lines       20481    20744     +263     
==========================================
- Hits        19445    18799     -646     
- Misses       1036     1945     +909

Flag	Coverage Δ
fast-tests	`90.62% <96.98%> (?)`
slow-tests	`?`

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

☔ View full report in Codecov by Harness.
📢 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.

hannahbaumann · 2026-06-02T09:12:02Z

pre-commit.ci autofix

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

hannahbaumann · 2026-06-02T09:34:13Z

pre-commit.ci autofix

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

…reeEnergy/openfe into structural_analysis_septop

hannahbaumann · 2026-06-10T14:13:53Z

As discussed offline, CI is currently breaking since the prev. openfe 1.11 results files for gather do not have the new analysis_settings. Should we turn them on by default to keep backward compatibility or remove the old test files and break backward compatibility? @IAlibay

jthorton · 2026-06-11T08:32:34Z

+        rdmol_A=d["rdmol_A"],
+        rdmol_B=d["rdmol_B"],
+        protein_selection="protein and name CA",
+        skip=None,


How about a test which checks the length of the output analysis changes in response to the skip value?

Added a test here: 396b353

Co-authored-by: Josh Horton <joshua.horton@openforcefield.org> Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>

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

hannahbaumann · 2026-06-11T09:10:30Z

pre-commit.ci autofix

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

…reeEnergy/openfe into structural_analysis_septop

IAlibay

overall looks good - just the one blocker for me re: settings

IAlibay · 2026-06-11T09:15:52Z

+  (complex leg only).
+* **Protein 2D RMSD** — pairwise RMSD matrix between all analyzed frames (complex leg only).
+
+Results are saved as an NPZ file (``structural_analysis.npz``) and plots are generated


[nit] can you link to https://numpy.org/doc/stable/reference/generated/numpy.savez.html in case someone doessn't know what NPZ is.

Added this!

IAlibay · 2026-06-11T09:17:50Z

    ThermoSettings,
 )
 from openfe.protocols.openmm_md.plain_md_methods import PlainMDSimulationUnit
+from openfe.protocols.openmm_septop.equil_septop_settings import MultiStateAnalysisSettings


Can you import directly from omm_settings or does it cause a circular import?

IAlibay · 2026-06-11T09:28:25Z

+                        rdmol_B,
+                    )
+
+        except Exception as e:


That's fair - I'd open an issue and put a todo here for now then.

IAlibay · 2026-06-11T09:33:18Z

+        ligand_B_indices: list[int],
+        smc_A: SmallMoleculeComponent,
+        smc_B: SmallMoleculeComponent,
+        analysis_settings: MultiStateAnalysisSettings,


Rather than passing it directly, can you just access it via _get_settings? That way you won't have two sets of settings around.

Yes, changed this!

IAlibay · 2026-06-11T09:35:24Z

        trajectory = simulation.outputs["trajectory"]
        checkpoint = simulation.outputs["checkpoint"]

+        alchem_comps = self._inputs["alchemical_components"]


That's odd - something we can work out in a separate issue / PR - could you raise an issue about setup.inputs being empty? That seems unintuitive.

IAlibay · 2026-06-11T09:36:16Z

+        # Get the analysis settings
+        settings = self._get_settings()
+        analysis_settings = settings["analysis_settings"]


Can we do this in run to match the other units / general pattern across the protocols?

Moved this into run!

IAlibay · 2026-06-11T09:39:27Z

+    skip: int | None = None
+    """
+    Frame stride for structural analysis. If ``None``, a stride is
+    chosen such that approximately (max.) 500 frames are analyzed per state.


Might be good to open an issue to work out if this is something we want to keep around in the future - at least it seems rather unintuitive to not just use all the data we have on hand.

Raised an issue here: #1993

hannahbaumann · 2026-06-11T13:43:57Z

pre-commit.ci autofix

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

github-actions · 2026-06-11T13:45:55Z

No API break detected ✅

First pass at adding structural analysis to the SepTop protocol

ea6e58a

hannahbaumann marked this pull request as draft May 28, 2026 10:05

hannahbaumann changed the title ~~Add structural analysis to the SepTop protocol~~ [WIP] Add structural analysis to the SepTop protocol May 28, 2026

pre-commit-ci Bot and others added 3 commits May 28, 2026 10:06

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

b0538ac

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

Small fix

0ae11d9

Small fix

00d3066

hannahbaumann self-assigned this May 28, 2026

hannahbaumann added 3 commits May 29, 2026 09:43

Fix ligand indices mapping

14381f9

Fix complex alignment

811a1b6

Small fix

54d4839

Add ligand indices to test

e1c101e

hannahbaumann added 2 commits June 1, 2026 15:40

Some updates

14ce7fd

Add tests for structural analysis

d953848

hannahbaumann marked this pull request as ready for review June 2, 2026 09:12

pre-commit-ci Bot and others added 2 commits June 2, 2026 09:12

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

850b1a5

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

fix mypy

5394785

pre-commit-ci Bot and others added 2 commits June 2, 2026 09:34

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

62a34dc

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

Update env

d1e3abe

hannahbaumann requested review from IAlibay and jthorton June 2, 2026 12:41

jameseastwood assigned IAlibay and unassigned hannahbaumann Jun 3, 2026

jthorton reviewed Jun 3, 2026

View reviewed changes

Comment thread src/openfe/protocols/openmm_septop/base_units.py Outdated

jthorton reviewed Jun 3, 2026

View reviewed changes

Comment thread src/openfe/protocols/openmm_septop/base_units.py Outdated

jthorton reviewed Jun 3, 2026

View reviewed changes

Comment thread src/openfe/protocols/openmm_septop/base_units.py Outdated

hannahbaumann and others added 8 commits June 10, 2026 10:30

add news entry

f66a158

try fixing docs

cda27f8

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

7924fbf

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

fix imports

f571b35

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

49a1b43

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

Add new settings to result example

a90cadd

Merge branch 'structural_analysis_septop' of https://github.com/OpenF…

8210dcd

…reeEnergy/openfe into structural_analysis_septop

Update SepTop user guide to add strucural analysis

54b7c53

hannahbaumann requested review from IAlibay and jthorton June 10, 2026 11:13

Update septop results for gather test

4207d4a

jthorton reviewed Jun 11, 2026

View reviewed changes

Comment thread docs/guide/protocols/septop.rst Outdated

jthorton reviewed Jun 11, 2026

View reviewed changes

Comment thread docs/guide/protocols/septop.rst Outdated

IAlibay reviewed Jun 11, 2026

View reviewed changes

Comment thread src/openfe/protocols/openmm_septop/equil_septop_settings.py Outdated

hannahbaumann and others added 3 commits June 11, 2026 11:02

Apply suggestions from code review

cb94126

Co-authored-by: Josh Horton <joshua.horton@openforcefield.org> Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>

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

d8dc40c

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

add missing import

f4d8702

pre-commit-ci Bot and others added 4 commits June 11, 2026 09:10

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

80aef7c

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

Add test for skip

396b353

Merge branch 'structural_analysis_septop' of https://github.com/OpenF…

82558d5

…reeEnergy/openfe into structural_analysis_septop

Update tests

ec7d96a

IAlibay requested changes Jun 11, 2026

View reviewed changes

hannahbaumann and others added 2 commits June 11, 2026 15:37

Address review comments

7b83f5d

Merge branch 'main' into structural_analysis_septop

fa7be25

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

a992d84

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

Conversation

hannahbaumann commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Developers certificate of origin

Uh oh!

hannahbaumann commented May 28, 2026

Uh oh!

hannahbaumann commented May 29, 2026

Uh oh!

codecov Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hannahbaumann commented Jun 2, 2026

Uh oh!

hannahbaumann commented Jun 2, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hannahbaumann commented Jun 10, 2026

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!

hannahbaumann commented Jun 11, 2026

Uh oh!

IAlibay left a comment

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!

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!

hannahbaumann commented Jun 11, 2026

Uh oh!

github-actions Bot commented Jun 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hannahbaumann commented May 28, 2026 •

edited

Loading

codecov Bot commented May 29, 2026 •

edited

Loading