Skip to content
Open
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
93d39f5
added prolif in environment.yml
talagayev Feb 17, 2026
5f48f14
black format
talagayev Feb 17, 2026
2ae6357
modified for AnalysisBase
talagayev Feb 25, 2026
976722d
black formatting
talagayev Feb 25, 2026
abed811
added tests
talagayev Feb 26, 2026
47060fb
Merge branch 'main' into prolif_class_implementation
talagayev Feb 26, 2026
445feb8
Added 2D visualizationa and adressed comments
talagayev Apr 15, 2026
a673636
Added tests
talagayev Apr 15, 2026
bf4723a
added args for selection
talagayev Apr 15, 2026
3e9a2b4
black formatting
talagayev Apr 15, 2026
76b87f4
Merge branch 'main' into prolif_class_implementation
talagayev Apr 15, 2026
990c86f
adjusted test error
talagayev Apr 21, 2026
d964589
Merge branch 'prolif_class_implementation' of https://github.com/Open…
talagayev Apr 21, 2026
23e218b
adjusted tests
talagayev Apr 21, 2026
2d1f22e
adjusted code to add additional visualization and changed the wrapper
talagayev May 13, 2026
81a3fa9
Merge branch 'main' into prolif_class_implementation
talagayev May 27, 2026
15e8620
Merge branch 'main' into prolif_class_implementation
hannahbaumann May 28, 2026
76037b5
Merge branch 'main' into prolif_class_implementation
talagayev Jun 11, 2026
0d8779a
Moving plots to plotting
talagayev Jun 18, 2026
a210c3f
Adjusting style
talagayev Jun 18, 2026
7ee7b5f
Merge branch 'main' of https://github.com/OpenFreeEnergy/openfe_analy…
talagayev Jul 1, 2026
f0607af
Added dependency
talagayev Jul 8, 2026
b4c1d4c
Moved plots and adressed issues
talagayev Jul 8, 2026
6702ad5
Adding tests and fixing issues
talagayev Jul 8, 2026
dabbbc5
Merge branch 'main' into prolif_class_implementation
hannahbaumann Jul 9, 2026
41f5798
addition of new tests
talagayev Jul 26, 2026
8131a28
Merge branch 'prolif_class_implementation' of https://github.com/Open…
talagayev Jul 26, 2026
793761f
Merge branch 'main' into prolif_class_implementation
hannahbaumann Aug 10, 2026
a9c6236
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 10, 2026
712b86c
Merge branch 'main' into prolif_class_implementation
hannahbaumann Aug 13, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ dependencies:
- openff-units
- pip
- tqdm
- prolif
Comment thread
hannahbaumann marked this conversation as resolved.
- pyyaml
# for testing
- coverage
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ dependencies = [
'spyrmsd',
'rdkit',
'matplotlib',
'prolif',
]
description="Trajectory analysis of free energy calculations."
readme="README.md"
Expand Down
248 changes: 248 additions & 0 deletions src/openfe_analysis/prolif.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,248 @@
from __future__ import annotations

import warnings
from typing import Any, Dict, Literal, Optional, Sequence, Tuple

import MDAnalysis as mda
import numpy as np
import prolif as plf

from .utils.universe_utils import guess_ligand_bonds


class ProLIFAnalysis:

@IAlibay IAlibay Feb 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you subclass AnalysisBase here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yup should be doable, will look into it :)

Wanted to wait and see how RMSD will look like to use an identical structure and have this initial PR to have bulletpoints to discuss tomorrow during the meeting and also have an easier overview on what was changed etc. :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We leave it as is for now and will change this when Prolif implements per frame analysis.

"""
ProLIF interaction fingerprint analysis for an OpenFEReader Universe.

Parameters
----------
universe
MDAnalysis Universe containing topology and trajectory.
ligand_ag
mda.AtomGroup representing the ligand.
water_order
Maximum WaterBridge interaction order (water-water interaction).
Only used if "WaterBridge" is tracked.
protein_cutoff
Distance cutoff in angstrom used to define the protein pocket
around the ligand.
water_cutoff
Distance cutoff in angstrom used to define waters considered
around the ligand/protein pocket.
interactions
Which interactions to track:
- None: ProLIF defaults (Hydrophobic, HBDonor, HBAcceptor,
PiStacking, Anionic, Cationic, CationPi, PiCation, VdWContact);
see ProLIF's ``DEFAULT_INTERACTIONS``:
https://github.com/chemosim-lab/ProLIF/blob/6d993eb1b54cd20cc160461dba1ee5e775cb4037/prolif/fingerprint.py#L97
- "all": every available interaction, including bridged ones
(e.g. WaterBridge)
- Sequence[str]: explicit list like ["VdWContact", "HBDonor"]
guess_bonds
If True, guess bonds for (protein, ligand, water) so ProLIF can
recognize donors/acceptors and bonded hydrogens.
"""

def __init__(
self,
universe: mda.Universe,
ligand_ag: mda.AtomGroup,
water_order: int = 3,
protein_cutoff: float = 12.0,
water_cutoff: float = 8.0,
interactions: Optional[Sequence[str] | str] = None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
interactions: Optional[Sequence[str] | str] = None,
interactions: Sequence[str] | Literal["all"] | None,

guess_bonds: bool = True,
) -> None:
self.universe = universe
self.ligand_ag = ligand_ag
self.water_order = water_order

self.frames: Optional[np.ndarray] = None
self.times: Optional[np.ndarray] = None
self.n_frames: Optional[int] = None

if guess_bonds:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's a lot happening in init here, maybe it would make sense to put some of these into private functions (e.g. to guess bonds, to build the fingerprints) so that they are easier to test separately and for a better overview of the different things that are going on.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

agree, can split it up a little bit, since it is quite crowded currently

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@talagayev i created a small utils function for the guessing of bonds here :

as part of this PR #92
Please let me know what you think about it!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@hannahbaumann looks good to me, will adjust the code to import it from there :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is merged now, so feel to try out the utils function here.

self._guess_bonds()

self._setup_selections(protein_cutoff, water_cutoff)

self.fp = self._build_fingerprint(interactions)

def _guess_bonds(self) -> None:
"""
Guess bonds for the protein, ligand and water selections in-place so
RDKit/ProLIF can detect donors/acceptors and bonded hydrogens.
"""
# Protein: guess on the full protein so any pocket residue later has bonds
guess_ligand_bonds(self.universe.select_atoms("protein"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it make sense to rename the function in utils to just guess_bonds or so? since the ligand part may be confusing for the protein and water.


# Ligand: stable group
guess_ligand_bonds(self.ligand_ag)

# Water: only if water-mediated interactions are of interest
water = self.universe.select_atoms("water")
if water.n_atoms:
guess_ligand_bonds(water)

def _setup_selections(self, protein_cutoff: float, water_cutoff: float) -> None:
"""
Build the updating pocket and water selections around the ligand.
"""
self.protein_ag = self.universe.select_atoms(
f"protein and byres around {protein_cutoff} group ligand",
ligand=self.ligand_ag,
updating=True,
)
self.water_ag = self.universe.select_atoms(
f"water and byres around {water_cutoff} (group ligand or group pocket)",
ligand=self.ligand_ag,
pocket=self.protein_ag,
updating=True,
)

def _build_fingerprint(self, interactions: Optional[Sequence[str] | str]) -> plf.Fingerprint:
"""
Resolve the requested interactions and construct the ProLIF Fingerprint.

Configures WaterBridge parameters when it is requested and waters are
present, and drops WaterBridge (with a warning) when they are not.
"""
available = plf.Fingerprint.list_available(show_bridged=True)

fp_interactions: Optional[list[str]]
if interactions is None:
fp_interactions = None

elif interactions == "all":
# ProLIF's "all" excludes bridged interactions (e.g. WaterBridge)
fp_interactions = list(available)

else:
fp_interactions = list(interactions)

self._parameters: Optional[dict] = None
if fp_interactions is not None and "WaterBridge" in fp_interactions:
if self.water_ag.n_atoms == 0:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This probably means that the WaterBridge will be removed if no waters are present at the initial frame, correct? Would there also be a scenario where a user would still want to run this analysis to see if waters come in later in the simulation, or is that generally not possible? Would it make sense to just generally check if waters are present?

warnings.warn(
"WaterBridge selected but water selection is empty at the initial "
"frame; removing WaterBridge from the requested interactions.",
UserWarning,
stacklevel=3,
)
fp_interactions = [
interaction for interaction in fp_interactions if interaction != "WaterBridge"
]
else:
self._parameters = {
"WaterBridge": {"water": self.water_ag, "order": self.water_order}
}

if not fp_interactions:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This means that probably both for the None case and a emty list case [], it would return the default Prolif interaction classes. If e.g. only WaterBridge was requested, but it can't find a water selection, that would be removed from the list and it would return all the default Prolif analysis een though they would not requested. Maybe specifically check for None here and potentially raise an error if it gets an empty list, saying that no fp can be computed?

return plf.Fingerprint(parameters=self._parameters)
return plf.Fingerprint(
interactions=fp_interactions,
parameters=self._parameters,
)

def run(
Comment thread
hannahbaumann marked this conversation as resolved.
self,
*,
start: Optional[int] = None,
stop: Optional[int] = None,
step: Optional[int] = None,
residues: Optional[Literal["all"] | Sequence[str | int]] = None,
progress: bool = True,
n_jobs: Optional[int] = None,
parallel_strategy: Optional[Literal["chunk", "queue"]] = None,
converter_kwargs: Optional[Tuple[Dict[str, Any], Dict[str, Any]]] = None,
) -> "ProLIFAnalysis":
"""
Run the fingerprint calculation over a slice of the trajectory.

Parameters
----------
start, stop, step
Trajectory slicing parameters.
residues
Passed to ProLIF: ``"all"`` to track every residue, or an explicit
sequence of residue identifiers. If None, ProLIF's default is used
and interactions with atoms are identified.
progress
Show progress bar.
n_jobs
Number of workers for parallel execution.).
parallel_strategy
ProLIF parallel strategy. If None, this wrapper sets:
- "chunk" for n_jobs None/1
- "queue" for n_jobs > 1
converter_kwargs
Two dicts: (ligand_kwargs, protein_kwargs) forwarded to the MDAnalysis→RDKit
converter. If None, we default to:
- ligand: {"inferrer": None, "implicit_hydrogens": False} (avoid valence issues)
- protein: {"implicit_hydrogens": False} (use topology bonds)

Returns
-------
self
Returned for fluent chaining.
"""
# Due to FEReader trajectory only certain strategies work with the format
if parallel_strategy is None:
# avoid ProLIF trying to pickle FEReader/netCDF trajectory to auto-pick strategy
parallel_strategy = "chunk" if (n_jobs is None or n_jobs == 1) else "queue"

_slice = slice(start, stop, step)
traj = self.universe.trajectory[_slice]

try:
n_total = len(self.universe.trajectory)
s0, s1, s2 = _slice.indices(n_total)
self.frames = np.arange(s0, s1, s2, dtype=int)
self.n_frames = len(traj)

if (
hasattr(self.universe.trajectory, "times")
and self.universe.trajectory.times is not None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does it ever have the .times attribute? I couldn't find it, but maybe I just missed it! Do you have a test in place that verifies this branch?

):
self.times = np.asarray(self.universe.trajectory.times)[self.frames]
elif getattr(self.universe.trajectory, "dt", None) is not None:
self.times = self.frames * self.universe.trajectory.dt
else:
self.times = None
except Exception:
self.frames = None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we really need the try/except here? What happens if this was removed?

self.times = None
self.n_frames = None

if converter_kwargs is None:
# Avoid Valence errors
converter_kwargs = (
{"inferrer": None, "implicit_hydrogens": False}, # ligand
{"implicit_hydrogens": False}, # protein
)

self.fp.run(
traj,
self.ligand_ag,
self.protein_ag,
residues=residues,
converter_kwargs=converter_kwargs,
progress=progress,
n_jobs=n_jobs,
parallel_strategy=parallel_strategy,
)

return self

@property
def ifp(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe add type hint here of what the return would be.

"""
Convenience accessor for underlying ProLIF fingerprint results.
"""
return getattr(self.fp, "ifp", None)

def to_dataframe(self, **kwargs):
"""
Transform fingerprint results to pd.DataFrame.
"""
return self.fp.to_dataframe(**kwargs)
Loading
Loading