Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d7dcd14
add charge validation
jthorton Aug 11, 2026
84174f3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 11, 2026
3c76241
add tests
jthorton Aug 11, 2026
2874599
Merge branch 'main' into charge_validation
jthorton Aug 14, 2026
36784f9
add validation to all protocols, remove runtime charge generation
jthorton Aug 14, 2026
251474c
fix tests
jthorton Aug 17, 2026
ba3f47d
Merge branch 'main' into charge_validation
jthorton Aug 17, 2026
045954d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 17, 2026
6c0d7d4
fix protocol charge generation tests
jthorton Aug 17, 2026
960b874
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 17, 2026
96b5d24
fix tests with missing charges
jthorton Aug 17, 2026
e2099ed
Merge branch 'main' into charge_validation
jthorton Aug 18, 2026
b8f7ecb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 18, 2026
98152db
speedup smoke test
jthorton Aug 18, 2026
df24ca1
add mixed charge/uncharged system testing
jthorton Aug 18, 2026
7dfdb3f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 18, 2026
0c8e7dd
switch to using sdfs
jthorton Aug 19, 2026
540aefa
remove extra file
jthorton Aug 19, 2026
bacb1c3
fix cli tests
jthorton Aug 19, 2026
9cb4322
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 19, 2026
7b7e4ee
add charges to inputs, remove slow mark
jthorton Aug 20, 2026
c43dbf0
Update src/openfe/tests/protocols/test_openmmutils.py
jthorton Aug 20, 2026
6c32c99
add news, update error to list all molecules with missing charges.
jthorton Aug 20, 2026
4cf822d
catch missing librarycharges handler in ff
jthorton Aug 21, 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
23 changes: 23 additions & 0 deletions news/charge_validation.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* <news item>

**Changed:**

* All protocols will raise a ``ProtocolValidationError`` error if a non-deterministic partial charge generation method is used at run time, please provide molecules with pre-calculated charges to use ``am1bcc`` or use a deterministic method such as library charges or AshGC.

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
33 changes: 1 addition & 32 deletions src/openfe/protocols/openmm_afe/base_afe_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
)
from openfe.protocols.openmm_md.plain_md_methods import PlainMDSimulationUnit
from openfe.protocols.openmm_utils import (
charge_generation,
multistate_analysis,
omm_compute,
settings_validation,
Expand Down Expand Up @@ -349,33 +348,6 @@ def _pre_equilibrate(

return equilibrated_positions, to_openmm(from_openmm(box))

@staticmethod
def _assign_partial_charges(
partial_charge_settings: OpenFFPartialChargeSettings,
small_mols: dict[SmallMoleculeComponent, OFFMolecule],
) -> None:
"""
Assign partial charges to the OpenFF Molecules associated with
all the SmallMoleculeComponents in the transformation.

Parameters
----------
charge_settings : OpenFFPartialChargeSettings
Settings for controlling how the partial charges are assigned.
small_mols : dict[SmallMoleculeComponent, openff.toolkit.Molecule]
Dictionary of OpenFF Molecules to add, keyed by their
associated SmallMoleculeComponent.
"""
for mol in small_mols.values():
charge_generation.assign_offmol_partial_charges(
offmol=mol,
overwrite=False,
method=partial_charge_settings.partial_charge_method,
toolkit_backend=partial_charge_settings.off_toolkit_backend,
generate_n_conformers=partial_charge_settings.number_of_conformers,
nagl_model=partial_charge_settings.nagl_model,
)

@staticmethod
def _get_system_generator(
settings: dict[str, SettingsBaseModel],
Expand All @@ -391,7 +363,7 @@ def _get_system_generator(
----------
settings : dict[str, SettingsBaseModel]
A dictionary of settings object for the unit.
solvent_comp : BaseSolventComponent | None
solvent_component : BaseSolventComponent | None
The solvent component of this system, if there is one.
openff_molecules : list[openff.toolkit.Molecule] | None
A list of OpenFF Molecules to generate templates for, if any.
Expand Down Expand Up @@ -704,9 +676,6 @@ def run(
# Get settings
settings = self._get_settings()

# Assign partial charges now to avoid any discrepancies later
self._assign_partial_charges(settings["charge_settings"], small_mols)

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.

I've been thinking more about this one - we're silently removing this, but won't users get caught out if they do use a deterministic charge method in the partial charge settings, e.g. NAGL?

@jthorton jthorton Aug 21, 2026

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.

Yeah that would be the downside, users would have to either use a ff with a NAGL handler or pre-charge them. If we keep support for NAGL and espaloma charge methods at runtime only that could work with the default setting being forcefield. It could be confusing if users have to define the forcefield to use in two places 1) the charges setting object and 2) small molecule force field, and then what happens if these are different?

Ideally I would love to keep this as simple as possible for users, we already encourage charging before running so moving to openfe will do no runtime charge generation doesn't feel like too much of a change?


# Get OpenMM topology, positions, system, and comp_resids
omm_topology, omm_system, positions, comp_resids = self._get_omm_objects(
settings=settings,
Expand Down
5 changes: 5 additions & 0 deletions src/openfe/protocols/openmm_afe/equil_binding_afe_method.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,11 @@ def _validate(
system_validation.validate_chemical_system(stateB)
self._validate_endstates(stateA, stateB)

# validate small molecule charges
small_ff = self.settings.forcefield_settings.small_molecule_forcefield
system_validation.validate_nondeterministic_charges(stateA, small_ff)
system_validation.validate_nondeterministic_charges(stateB, small_ff)

# Validate the complex lambda schedule
self._validate_lambda_schedule(
self.settings.complex_lambda_settings,
Expand Down
5 changes: 5 additions & 0 deletions src/openfe/protocols/openmm_afe/equil_solvation_afe_method.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,11 @@ def _validate(
system_validation.validate_chemical_system(stateB)
self._validate_endstates(stateA, stateB)

# validate small molecule charges
small_ff = self.settings.solvent_forcefield_settings.small_molecule_forcefield
system_validation.validate_nondeterministic_charges(stateA, small_ff)
system_validation.validate_nondeterministic_charges(stateB, small_ff)

# Validate the lambda schedule
for solv_sets in (
self.settings.solvent_simulation_settings,
Expand Down
34 changes: 4 additions & 30 deletions src/openfe/protocols/openmm_md/plain_md_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
PlainMDProtocolSettings,
)
from openfe.protocols.openmm_utils import (
charge_generation,
omm_compute,
serialization,
settings_validation,
Expand Down Expand Up @@ -197,6 +196,10 @@ def _validate(
# Validate the ChemicalSystem
system_validation.validate_chemical_system(stateA)

# Validate small molecule charges
small_ff = self.settings.forcefield_settings.small_molecule_forcefield
system_validation.validate_nondeterministic_charges(stateA, small_ff)

# Validate solvent component if present
nonbond = self.settings.forcefield_settings.nonbonded_method
system_validation.validate_solvent(stateA, nonbond)
Expand Down Expand Up @@ -333,32 +336,6 @@ class PlainMDSetupUnit(PlainMDUnitMixin, gufe.ProtocolUnit):
Protocol setup unit for plain MD simulations which handles charging, system building and solvation.
"""

@staticmethod
def _assign_partial_charges(
charge_settings: OpenFFPartialChargeSettings,
smc_components: dict[SmallMoleculeComponent, OFFMolecule],
) -> None:
"""
Assign partial charges to SMCs.

Parameters
----------
charge_settings : OpenFFPartialChargeSettings
Settings for controlling how the partial charges are assigned.
smc_components : dict[SmallMoleculeComponent, openff.toolkit.Molecule]
Dictionary of OpenFF Molecules to add, keyed by
SmallMoleculeComponent.
"""
for mol in smc_components.values():
charge_generation.assign_offmol_partial_charges(
offmol=mol,
overwrite=False,
method=charge_settings.partial_charge_method,
toolkit_backend=charge_settings.off_toolkit_backend,
generate_n_conformers=charge_settings.number_of_conformers,
nagl_model=charge_settings.nagl_model,
)

def run(
self,
*,
Expand Down Expand Up @@ -450,9 +427,6 @@ def run(
i: i.to_openff() for i in small_mols
}

# a. assign partial charges to smcs
self._assign_partial_charges(charge_settings, smc_components)

# b. get a system generator
if output_settings.forcefield_cache is not None:
ffcache = self.shared_basepath / output_settings.forcefield_cache
Expand Down
5 changes: 5 additions & 0 deletions src/openfe/protocols/openmm_rfe/hybridtop_protocols.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,11 @@ def _validate(
system_validation.validate_chemical_system(stateB)
self._validate_endstates(stateA, stateB)

# Validate small molecule charges
small_ff = self.settings.forcefield_settings.small_molecule_forcefield
system_validation.validate_nondeterministic_charges(stateA, small_ff)
system_validation.validate_nondeterministic_charges(stateB, small_ff)

# Validate the mapping
alchem_comps = system_validation.get_alchemical_components(stateA, stateB)
self._validate_mapping(mapping, alchem_comps)
Expand Down
31 changes: 0 additions & 31 deletions src/openfe/protocols/openmm_rfe/hybridtop_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
from ...analysis import plotting
from ...utils import log_system_probe, without_oechem_backend
from ..openmm_utils import (
charge_generation,
multistate_analysis,
omm_compute,
settings_validation,
Expand Down Expand Up @@ -205,33 +204,6 @@ def _get_components(

return solvent_comp, protein_comp, small_mols

@staticmethod
def _assign_partial_charges(
charge_settings: OpenFFPartialChargeSettings,
small_mols: dict[SmallMoleculeComponent, OFFMolecule],
) -> None:
"""
Assign partial charges to the OpenFF Molecules associated with all
the SmallMoleculeComponents in the transformation.

Parameters
----------
charge_settings : OpenFFPartialChargeSettings
Settings for controlling how the partial charges are assigned.
small_mols : dict[SmallMoleculeComponent, openff.toolkit.Molecule]
Dictionary of OpenFF Molecules to add, keyed by
their associated SmallMoleculeComponent.
"""
for smc, mol in small_mols.items():
charge_generation.assign_offmol_partial_charges(
offmol=mol,
overwrite=False,
method=charge_settings.partial_charge_method,
toolkit_backend=charge_settings.off_toolkit_backend,
generate_n_conformers=charge_settings.number_of_conformers,
nagl_model=charge_settings.nagl_model,
)

@staticmethod
def _get_system_generator(
settings: dict[str, SettingsBaseModel],
Expand Down Expand Up @@ -761,9 +733,6 @@ def run(
# in the topology
alchem_resnames = [assigned[alchem_comps["stateA"][0]]]

# Assign partial charges now to avoid any discrepancies later
self._assign_partial_charges(settings["charge_settings"], small_mols)

(
stateA_system,
stateA_topology,
Expand Down
27 changes: 0 additions & 27 deletions src/openfe/protocols/openmm_septop/base_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
from openfe.utils import log_system_probe, without_oechem_backend

from ..openmm_utils import (
charge_generation,
multistate_analysis,
settings_validation,
system_creation,
Expand Down Expand Up @@ -457,32 +456,6 @@ def _get_system_generator(
)
return system_generator

@staticmethod
def _assign_partial_charges(
partial_charge_settings: OpenFFPartialChargeSettings,
smc_components: dict[SmallMoleculeComponent, OFFMolecule],
) -> None:
"""
Assign partial charges to OFFMolecules inplace.

Parameters
----------
charge_settings : OpenFFPartialChargeSettings
Settings for controlling how the partial charges are assigned.
smc_components : dict[SmallMoleculeComponent, openff.toolkit.Molecule]
Dictionary of OpenFF Molecules to add, keyed by
SmallMoleculeComponent.
"""
for mol in smc_components.values():
charge_generation.assign_offmol_partial_charges(
offmol=mol,
overwrite=False,
method=partial_charge_settings.partial_charge_method,
toolkit_backend=partial_charge_settings.off_toolkit_backend,
generate_n_conformers=partial_charge_settings.number_of_conformers,
nagl_model=partial_charge_settings.nagl_model,
)

def _get_modeller(
self,
protein_component: Optional[ProteinComponent],
Expand Down
5 changes: 5 additions & 0 deletions src/openfe/protocols/openmm_septop/equil_septop_method.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,11 @@ def _validate(
system_validation.validate_chemical_system(stateB)
self._validate_endstates(stateA, stateB)

# Validate small molecule charges
small_ff = self.settings.forcefield_settings.small_molecule_forcefield
system_validation.validate_nondeterministic_charges(stateA, small_ff)
system_validation.validate_nondeterministic_charges(stateB, small_ff)

# Validate the lambda schedule
self._validate_lambda_schedule(
self.settings.solvent_lambda_settings,
Expand Down
6 changes: 0 additions & 6 deletions src/openfe/protocols/openmm_septop/septop_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -711,9 +711,6 @@ def run(
# 3. Get settings
settings = self._get_settings()

# 4. Assign partial charges
self._assign_partial_charges(settings["charge_settings"], smc_comps_AB)

# 5. Get the OpenMM systems
omm_system_A, omm_topology_A, positions_A, modeller_A, comp_resids_A = (
self.get_system(
Expand Down Expand Up @@ -1080,9 +1077,6 @@ def run(
# 2. Get settings
settings = self._get_settings()

# 3. Assign partial charges
self._assign_partial_charges(settings["charge_settings"], smc_comps_AB)

# 4. Update the positions of ligand B:
# - solvent: Offset ligand B with respect to ligand A
offset = self._get_ligand_offset(
Expand Down
62 changes: 62 additions & 0 deletions src/openfe/protocols/openmm_utils/system_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
SolventComponent,
)
from gufe.components.errors import ComponentValidationError
from gufe.protocols.errors import ProtocolValidationError
from openff.toolkit import ForceField
from openff.toolkit import Molecule as OFFMol

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -371,3 +373,63 @@ def validate_chemical_system(system: ChemicalSystem):
except ComponentValidationError as e:
errmsg = f"Component {entry} from ChemicalSystem {system.name} failed validation: {e}"
raise ComponentValidationError(errmsg)


def validate_nondeterministic_charges(system: ChemicalSystem, small_molecule_forcefield: str):
"""
Validate that the SmallMoleculeComponents of the system will have deterministic partial charges.

This is determined by checking for charges on the molecules before checking what would be assigned by the force field.

Parameters
----------
system : ChemicalSystem
The ChemicalSystem to validate with SmallMoleculeComponents.
small_molecule_forcefield : str
The force field to be used for the SmallMoleculeComponents.

Raises
------
ProtocolValidationError
If any SmallMoleculeComponents in the system would have am1bcc charges generated at runtime.
"""
smcs: list[SmallMoleculeComponent] = system.get_components_of_type(SmallMoleculeComponent)
if "espaloma" in small_molecule_forcefield or "gaff" in small_molecule_forcefield:
# this will always generate charges at runtime, so raise an error for missing charges
ff = None
else:
try:
ff = ForceField(small_molecule_forcefield)
except OSError:
# try again but adding offxml to the end of the force field name if the user passed one of the installed force fields without the extension
try:
ff = ForceField(small_molecule_forcefield + ".offxml")
except OSError as e:
errmsg = f"Could not load force field {small_molecule_forcefield} or {small_molecule_forcefield}.offxml: {e}"
raise ProtocolValidationError(errmsg)

errors = []
for smc in smcs:
offmol = smc.to_openff()
if offmol.partial_charges is not None and np.any(offmol.partial_charges):

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 we want to support a case where the user supplies partial charges will all zeros?

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.

+1 - given we have our own benchmark case where we want to do this, we probably should support 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.

Yes good idea. Currently, this only works as a Library charge would we want to keep that as a source of protection to make sure users know what they are doing or allow it as charges on the molecule as well?

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.

That's a good point - I think if it's easily allowed by LibraryCharge then it might be better to keep the check in. There's too many ways someone could accidentally add partial charges.

continue

# check the labels assigned for an openff force field
if ff is not None:
labels = ff.label_molecules(offmol.to_topology())[0]
else:
# return a dummy label as the gaff and espaloma should always give am1bcc charges
labels = {"LibraryCharges": {}}

# We count library and nagl charges as deterministic
# While users could use a deterministic charge method with increments we don't currently support this
if not labels.get("LibraryCharges", {}) and "NAGLCharges" not in labels:
errors.append(smc)

if errors:
errmsg = (
f"System: '{system.name}' contains SmallMoleculeComponents which would have am1bcc charges generated at runtime which is non-deterministic. "
f"Please provide a molecule with pre-computed charges or use library charges instead. "
f"The following Components are affected: {', '.join([str(smc) for smc in errors])}"
)
raise ProtocolValidationError(errmsg)
Loading
Loading