diff --git a/src/openfe/protocols/openmm_rfe/_rfe_utils/topologyhelpers.py b/src/openfe/protocols/openmm_rfe/_rfe_utils/topologyhelpers.py index 7181f4ce5..c56e2e648 100644 --- a/src/openfe/protocols/openmm_rfe/_rfe_utils/topologyhelpers.py +++ b/src/openfe/protocols/openmm_rfe/_rfe_utils/topologyhelpers.py @@ -10,6 +10,7 @@ import itertools import logging import warnings +from collections import Counter from copy import deepcopy from typing import Optional, Union @@ -21,76 +22,208 @@ from openmm import NonbondedForce, System, app from openmm import unit as omm_unit -from openfe import SolventComponent - logger = logging.getLogger(__name__) -def _get_ion_and_water_parameters( +def _get_ion_parameters_from_forcefield( + forcefield: app.ForceField, + ion_resname: str, + ion_element: app.Element, +) -> tuple: + """ + Get NonbondedForce parameters for a bare monovalent ion by creating a + minimal single-ion system via the ForceField. Used as a fallback when + no ion of the required type is present in the topology. + + Parameters + ---------- + forcefield : app.ForceField + The ForceField to use to parameterize the ion. + ion_resname : str + The residue name of the ion to parameterize (e.g. 'NA', 'CL'). + ion_element : app.Element + The element of the ion. + + Returns + ------- + ion_charge : openmm.unit.Quantity + The partial charge of the ion. + ion_sigma : openmm.unit.Quantity + The NonbondedForce sigma parameter of the ion. + ion_epsilon : openmm.unit.Quantity + The NonbondedForce epsilon parameter of the ion. + """ + dummy_top = app.Topology() + res = dummy_top.addResidue(ion_resname, dummy_top.addChain()) + dummy_top.addAtom(ion_resname, ion_element, res) + dummy_system = forcefield.createSystem(dummy_top) + + nbf = [i for i in dummy_system.getForces() + if isinstance(i, NonbondedForce)][0] + + return nbf.getParticleParameters(0) + + +def _get_water_parameters( topology: app.Topology, system: System, - ion_resname: str, water_resname: str = 'HOH', -): +) -> tuple: """ - Get ion, and water (oxygen and hydrogen) atoms parameters. + Get water oxygen and hydrogen partial charges from the system. Parameters ---------- topology : app.Topology - The topology to search for the ion and water + The topology to search for water atoms. system : app.System The system associated with the input topology object. - ion_resname : str - The residue name of the ion to get parameters for water_resname : str - The residue name of the water to get parameters for. Default 'HOH'. + The residue name of the water. Default 'HOH'. Returns ------- - ion_charge : float - The partial charge of the ion atom - ion_sigma : float - The NonbondedForce sigma parameter of the ion atom - ion_epsilon : float - The NonbondedForce epsilon parameter of the ion atom - o_charge : float + o_charge : openmm.unit.Quantity The partial charge of the water oxygen. - h_charge : float + h_charge : openmm.unit.Quantity The partial charge of the water hydrogen. Raises ------ ValueError - If there are no ``ion_resname`` or ``water_resname`` named residues in - the input ``topology``. + If no water residue named ``water_resname`` with O and H atoms + is found in the topology. + """ + nbf = [i for i in system.getForces() + if isinstance(i, NonbondedForce)][0] - Attribution - ----------- - Based on `perses.utils.charge_changing.get_ion_and_water_parameters`. + o_charge = None + h_charge = None + + for residue in topology.residues(): + if residue.name != water_resname: + continue + for atom in residue.atoms(): + if atom.element is None: + continue + charge, _, _ = nbf.getParticleParameters(atom.index) + if atom.element.symbol == 'O': + o_charge = charge + elif atom.element.symbol == 'H': + h_charge = charge + if o_charge is not None and h_charge is not None: + break + + if o_charge is None or h_charge is None: + raise ValueError( + f"Could not find water residue '{water_resname}' with O and H " + "atoms in the topology. Water parameters are required for " + "alchemical charge correction." + ) + + return o_charge, h_charge + + +def _get_ion_parameters( + topology: app.Topology, + system: System, + charge_difference: int, + forcefield: app.ForceField, +) -> tuple: """ - def _find_atom(topology, resname, elementname): - for atom in topology.atoms(): - if atom.residue.name == resname: - if (elementname is None or atom.element.symbol == elementname): - return atom.index - errmsg = ("Error encountered when attempting to explicitly handle " - "charge changes using an alchemical water. No residue " - f"named: {resname} found, with element {elementname}") - raise ValueError(errmsg) + Get NonbondedForce parameters for a monovalent counter-ion to + compensate a charge-changing alchemical transformation. + + The ion is selected by scanning the topology for monatomic residues + whose charge matches the sign of the charge difference. Three + scenarios are handled: + + 1. **Monovalent ions present** \u2014 parameters are taken from the most + abundant matching ion in the topology (e.g. if the system has + both Na+ and K+, the more numerous species is used). + 2. **Only divalent or wrong-sign ions present** \u2014 these are skipped + by the monovalent filter (|q| \u2248 1) and we fall through to the + forcefield fallback. + 3. **No ions at all** (e.g. zero ionic strength, or a + SolvatedPDBComponent without ions) \u2014 a minimal single-ion + system is built from the forcefield to obtain NA or CL + parameters directly. - ion_index = _find_atom(topology, ion_resname, None) - oxygen_index = _find_atom(topology, water_resname, 'O') - hydrogen_index = _find_atom(topology, water_resname, 'H') + Parameters + ---------- + topology : app.Topology + The topology to search for the most abundant ion type. + system : app.System + The system associated with the input topology object. + charge_difference : int + The charge difference between state A and state B. Determines + whether to look for a positive or negative ion. + forcefield : app.ForceField + The force field to use for parameterization if no ion is found + in the topology. + + Returns + ------- + ion_charge : openmm.unit.Quantity + ion_sigma : openmm.unit.Quantity + ion_epsilon : openmm.unit.Quantity + Attribution + ----------- + Based on `perses.utils.charge_changing.get_ion_and_water_parameters` + and OpenFE PR #1978. + """ nbf = [i for i in system.getForces() if isinstance(i, NonbondedForce)][0] - ion_charge, ion_sigma, ion_epsilon = nbf.getParticleParameters(ion_index) - o_charge, _, _ = nbf.getParticleParameters(oxygen_index) - h_charge, _, _ = nbf.getParticleParameters(hydrogen_index) + # We need an ion whose charge sign matches the charge difference: + # a positive charge_difference means the system lost positive charge + # (stateA more positive), so we need a positive ion to compensate. + desired_sign = np.sign(charge_difference) + ion_counts: Counter = Counter() + ion_atom_indices: dict[str, int] = {} + + for residue in topology.residues(): + atoms = list(residue.atoms()) + # Monatomic residues are ion candidates. + if len(atoms) != 1: + continue + atom = atoms[0] + if atom.element is None: + continue + charge, _, _ = nbf.getParticleParameters(atom.index) + charge_val = charge.value_in_unit(omm_unit.elementary_charge) + is_monovalent = abs(abs(charge_val) - 1.0) < 0.01 + has_correct_sign = np.sign(charge_val) == desired_sign + if is_monovalent and has_correct_sign: + ion_counts[residue.name] += 1 + if residue.name not in ion_atom_indices: + ion_atom_indices[residue.name] = atom.index + + if ion_counts: + # Use parameters from the most abundant matching ion in the topology. + best_resname = ion_counts.most_common(1)[0][0] + return nbf.getParticleParameters(ion_atom_indices[best_resname]) + + # No matching ion in the topology — fall back to NA/CL from the forcefield. + if charge_difference > 0: + fallback_resname = 'NA' + fallback_element = app.Element.getByAtomicNumber(11) # Na + else: + fallback_resname = 'CL' + fallback_element = app.Element.getByAtomicNumber(17) # Cl + + wmsg = ( + f"No monovalent ion with the appropriate charge sign found in " + f"the topology. Defaulting to '{fallback_resname}' and obtaining " + f"parameters from the forcefield directly." + ) + warnings.warn(wmsg) + logger.warning(wmsg) - return ion_charge, ion_sigma, ion_epsilon, o_charge, h_charge + return _get_ion_parameters_from_forcefield( + forcefield, fallback_resname, fallback_element, + ) def _fix_alchemical_water_atom_mapping( @@ -123,13 +256,36 @@ def _fix_alchemical_water_atom_mapping( def handle_alchemical_waters( - water_resids: list[int], topology: app.Topology, - system: System, system_mapping: dict, + water_resids: list[int], + topology: app.Topology, + system: System, + system_mapping: dict, charge_difference: int, - solvent_component: SolventComponent, + forcefield: app.ForceField, + water_resname: str, ): """ - Add alchemical waters from a pre-defined list. + Correct for a net charge change by alchemically mutating water + molecules into monovalent ions. + + A water far from the solute is selected and its NonbondedForce + parameters are replaced: the oxygen receives the ion's charge, + sigma, and epsilon, while the hydrogen charges are zeroed out. + No bonded parameters are modified — this is safe because standard + rigid water models (TIP3P, SPC/E) use distance constraints + (SETTLE / SHAKE) rather than harmonic bond and angle forces. + If a flexible water model were used, the residual O-H bond and + H-O-H angle springs would create unphysical restoring forces on + the ghost hydrogens. + + Ion parameters are resolved in order of preference: + + 1. From the most abundant monovalent ion of the correct charge + sign already present in the topology (e.g. Na+ or K+). + 2. If only divalent or wrong-sign ions exist, they are skipped. + 3. If no suitable ion is found at all, parameters are obtained + by creating a minimal single-ion system from the forcefield + (defaults to NA / CL). Parameters ---------- @@ -143,14 +299,11 @@ def handle_alchemical_waters( A dictionary of system mappings between the stateA and stateB systems charge_difference : int The charge difference between state A and state B. - positive_ion_resname : str - The name of a positive ion to replace the water with if the absolute - charge difference is positive. - negative_ion_resname : str - The name of a negative ion to replace the water with if the absolute - charge difference is negative. + forcefield : app.ForceField + The forcefield to use for ion parameterization. Used as a fallback + when no suitable ion is present in the topology. water_resname : str - The residue name of the water to get parameters for. Default 'HOH'. + The residue name of the water to get parameters for. Raises ------ @@ -171,17 +324,14 @@ def handle_alchemical_waters( f"difference: {abs(charge_difference)}") raise ValueError(errmsg) - if charge_difference > 0: - ion_resname = solvent_component.positive_ion.strip('-+').upper() - elif charge_difference < 0: - ion_resname = solvent_component.negative_ion.strip('-+').upper() - # if there's no charge difference then just skip altogether - else: + if charge_difference == 0: return None - ion_charge, ion_sigma, ion_epsilon, o_charge, h_charge = _get_ion_and_water_parameters( - topology, system, ion_resname, - 'HOH', # Modeller always adds HOH waters + ion_charge, ion_sigma, ion_epsilon = _get_ion_parameters( + topology, system, charge_difference, forcefield, + ) + o_charge, h_charge = _get_water_parameters( + topology, system, water_resname, ) # get the nonbonded forces @@ -193,12 +343,15 @@ def handle_alchemical_waters( # for convenience just grab the first & only entry nbf = nbfrcs[0] - # Loop through residues, check if they match the residue index - # mutate the atom as necessary + # Mutate selected water residues into ions by replacing only their + # nonbonded parameters. The bonded topology is left untouched: + # this is correct for rigid water models (TIP3P, SPC/E) where O-H + # distances and H-O-H angles are enforced by SETTLE / SHAKE + # constraints, not by harmonic force terms. For flexible water + # models the leftover bond/angle springs would apply unphysical + # forces to the ghost hydrogens — those models are not supported. for res in topology.residues(): if res.index in water_resids: - # if the number of atoms > 3, then we have virtual sites which are - # not supported currently if len([at for at in res.atoms()]) > 3: errmsg = ("Non 3-site waters (i.e. waters with virtual sites) " "are not currently supported as alchemical waters") @@ -209,10 +362,12 @@ def handle_alchemical_waters( charge, sigma, epsilon = nbf.getParticleParameters(idx) _fix_alchemical_water_atom_mapping(system_mapping, idx) + # Oxygen → ion: replace charge, sigma, epsilon. if charge == o_charge: nbf.setParticleParameters( idx, ion_charge, ion_sigma, ion_epsilon ) + # Hydrogen → ghost: zero charge, keep LJ. else: if charge != h_charge: errmsg = ("modifying an atom that doesn't match known " @@ -433,7 +588,6 @@ def _remove_constraints(old_to_new_atom_map, old_system, old_topology, * Very slow, needs refactoring * Can we drop having topologies as inputs here? """ - from collections import Counter no_const_old_to_new_atom_map = deepcopy(old_to_new_atom_map) diff --git a/src/openfe/protocols/openmm_rfe/hybridtop_protocols.py b/src/openfe/protocols/openmm_rfe/hybridtop_protocols.py index ed83e0459..2d231f4b9 100644 --- a/src/openfe/protocols/openmm_rfe/hybridtop_protocols.py +++ b/src/openfe/protocols/openmm_rfe/hybridtop_protocols.py @@ -28,6 +28,7 @@ ProteinComponent, ProteinMembraneComponent, SmallMoleculeComponent, + SolvatedPDBComponent, SolventComponent, settings, ) @@ -372,7 +373,7 @@ def _validate_charge_difference( mapping: LigandAtomMapping, nonbonded_method: str, explicit_charge_correction: bool, - solvent_component: SolventComponent | None, + solvent_component: SolventComponent | SolvatedPDBComponent | None, ): """ Validates the net charge difference between the two states. @@ -385,8 +386,8 @@ def _validate_charge_difference( The OpenMM nonbonded method used for the simulation. explicit_charge_correction : bool Whether or not to use an explicit charge correction. - solvent_component : openfe.SolventComponent | None - The SolventComponent of the simulation. + solvent_component : SolventComponent | SolvatedPDBComponent | None + The solvent-bearing component of the simulation. Raises ------ @@ -435,13 +436,25 @@ def _validate_charge_difference( ) raise ValueError(errmsg) - ion = {-1: solvent_component.positive_ion, 1: solvent_component.negative_ion}[difference] + # SolventComponent carries explicit ion names; SolvatedPDBComponent + # (e.g. ProteinMembraneComponent) does not — the actual ion type will + # be resolved at runtime from the topology or forcefield. + if isinstance(solvent_component, SolventComponent): + positive_ion = solvent_component.positive_ion.strip("-+").upper() + negative_ion = solvent_component.negative_ion.strip("-+").upper() + ion = {-1: positive_ion, 1: negative_ion}[difference] + wmsg = ( + f"A charge difference of {difference} is observed " + "between the end states. This will be addressed by " + f"transforming a water into a {ion} ion" + ) + elif isinstance(solvent_component, SolvatedPDBComponent): + wmsg = ( + f"A charge difference of {difference} is observed " + "between the end states. This will be addressed by " + "transforming a water into an ion." + ) - wmsg = ( - f"A charge difference of {difference} is observed " - "between the end states. This will be addressed by " - f"transforming a water into a {ion} ion" - ) logger.info(wmsg) @staticmethod @@ -560,6 +573,18 @@ def _validate( # Note: validation depends on the mapping & solvent component checks if stateA.contains(SolventComponent): solv_comp = stateA.get_components_of_type(SolventComponent)[0] + elif stateA.contains(SolvatedPDBComponent): + # ProteinMembraneComponent (and other SolvatedPDBComponents) + # already contain explicit water and ions as part of the + # pre-equilibrated box. Treating them as solvated here lets + # _validate_charge_difference enable explicit charge correction + # (alchemical water → ion mutation) for charge-changing edges, + # which would otherwise fail with "Cannot use explicit charge + # correction without solvent". + # + # validate_solvent() (called above) guarantees at most one + # SolvatedPDBComponent, so [0] is safe. + solv_comp = stateA.get_components_of_type(SolvatedPDBComponent)[0] else: solv_comp = None diff --git a/src/openfe/protocols/openmm_rfe/hybridtop_units.py b/src/openfe/protocols/openmm_rfe/hybridtop_units.py index cd634f391..b076c912d 100644 --- a/src/openfe/protocols/openmm_rfe/hybridtop_units.py +++ b/src/openfe/protocols/openmm_rfe/hybridtop_units.py @@ -390,7 +390,7 @@ def _handle_net_charge( charge_difference: int, system_mappings: dict[str, dict[int, int]], distance_cutoff: Quantity, - solvent_component: SolventComponent | None, + forcefield: openmm.app.ForceField, ) -> None: """ Handle system net charge by adding an alchemical water. @@ -404,7 +404,9 @@ def _handle_net_charge( charge_difference : int system_mappings : dict[str, dict[int, int]] distance_cutoff : Quantity - solvent_component : SolventComponent | None + forcefield : openmm.app.ForceField + The forcefield used to parameterize the system. Needed to + obtain ion parameters when no ions are present in the topology. """ # Base case, return if no net charge if charge_difference == 0: @@ -425,7 +427,8 @@ def _handle_net_charge( system=stateB_system, system_mapping=system_mappings, charge_difference=charge_difference, - solvent_component=solvent_component, + forcefield=forcefield, + water_resname="HOH", ) def _get_omm_objects( @@ -545,6 +548,10 @@ def _filter_small_mols(smols, state): # Net charge: add alchemical water if needed # Must be done here as we in-place modify the particles of state B. if settings["alchemical_settings"].explicit_charge_correction: + # The forcefield from the system generator contains all the + # ion templates we need. We use state A's generator, but both + # states share the same forcefield parameters. + forcefield = states_inputs["A"]["generator"].forcefield self._handle_net_charge( stateA_topology=stateA_topology, stateA_positions=stateA_positions, @@ -553,7 +560,7 @@ def _filter_small_mols(smols, state): charge_difference=mapping.get_alchemical_charge_difference(), system_mappings=system_mappings, distance_cutoff=settings["alchemical_settings"].explicit_charge_correction_cutoff, - solvent_component=solvent_component, + forcefield=forcefield, ) # Finally get the state B positions diff --git a/src/openfe/tests/protocols/openmm_rfe/test_hybrid_top_protocol.py b/src/openfe/tests/protocols/openmm_rfe/test_hybrid_top_protocol.py index fb297e248..506b18b87 100644 --- a/src/openfe/tests/protocols/openmm_rfe/test_hybrid_top_protocol.py +++ b/src/openfe/tests/protocols/openmm_rfe/test_hybrid_top_protocol.py @@ -1860,7 +1860,7 @@ def benzene_solvent_openmm_system(benzene_modifications): positions = to_openmm(from_openmm(modeller.getPositions())) system = system_generator.create_system(topology, molecules=[offmol]) - return system, topology, positions + return system, topology, positions, system_generator.forcefield @pytest.fixture(scope="session") @@ -1900,7 +1900,7 @@ def benzene_tip4p_solvent_openmm_system(benzene_modifications): positions = to_openmm(from_openmm(modeller.getPositions())) system = system_generator.create_system(topology, molecules=[offmol]) - return system, topology, positions + return system, topology, positions, system_generator.forcefield @pytest.fixture @@ -1910,7 +1910,7 @@ def benzene_self_system_mapping(benzene_solvent_openmm_system): alchemical transformation (this technically doesn't work in practice because the RFE protocol expects an alchemical component). """ - system, topology, positions = benzene_solvent_openmm_system + system, topology, positions, forcefield = benzene_solvent_openmm_system res = [r for r in topology.residues()] benzene_res = [r for r in res if r.name == "UNK"][0] @@ -1932,28 +1932,124 @@ def benzene_self_system_mapping(benzene_solvent_openmm_system): return system_mapping -@pytest.mark.parametrize( - "ion, water", - [ - ["NA", "SOL"], - ["NX", "WAT"], - ], -) -def test_get_ion_water_parameters_unknownresname(ion, water, benzene_solvent_openmm_system): - system, topology, positions = benzene_solvent_openmm_system +def test_get_water_parameters_unknown_resname(benzene_solvent_openmm_system): + """Water lookup should fail when the water residue name doesn't match.""" + system, topology, positions, forcefield = benzene_solvent_openmm_system - errmsg = "Error encountered when attempting to explicitly handle" + errmsg = "Could not find water residue" with pytest.raises(ValueError, match=errmsg): - topologyhelpers._get_ion_and_water_parameters( - topology, system, ion_resname=ion, water_resname=water + topologyhelpers._get_water_parameters( + topology, system, water_resname="SOL" + ) + + +def test_get_ion_parameters_from_topology(benzene_solvent_openmm_system): + """ + Scenario 1: monovalent ions with the correct sign are present in + the topology — parameters should come from the most abundant one. + """ + system, topology, positions, forcefield = benzene_solvent_openmm_system + + # The benzene system is solvated with NaCl at default concentration, + # so Na+ ions should be present in the topology. + ion_charge, ion_sigma, ion_epsilon = topologyhelpers._get_ion_parameters( + topology, system, charge_difference=1, forcefield=forcefield, + ) + + assert ion_charge.value_in_unit(omm_unit.elementary_charge) == pytest.approx(1.0) + + +def test_get_ion_parameters_skips_divalent(benzene_solvent_openmm_system): + """ + Scenario 2: only divalent (or wrong-sign) ions are present — should + fall back to the forcefield. + + We construct this by stripping all monovalent cations from a copy + of the system (zeroing their charges) so none pass the filter. + """ + system, topology, positions, forcefield = benzene_solvent_openmm_system + + new_system = copy.deepcopy(system) + nbf = [f for f in new_system.getForces() if isinstance(f, NonbondedForce)][0] + + # Zero out all monovalent positive charges (Na+) so the scanner + # finds nothing suitable. + for residue in topology.residues(): + atoms = list(residue.atoms()) + if len(atoms) != 1: + continue + atom = atoms[0] + if atom.element is None: + continue + charge, sigma, epsilon = nbf.getParticleParameters(atom.index) + charge_val = charge.value_in_unit(omm_unit.elementary_charge) + if abs(charge_val - 1.0) < 0.01: + # Set to a divalent charge so it won't match the monovalent filter + nbf.setParticleParameters( + atom.index, + 2.0 * omm_unit.elementary_charge, + sigma, epsilon, + ) + + with pytest.warns(UserWarning, match="No monovalent ion"): + ion_charge, ion_sigma, ion_epsilon = topologyhelpers._get_ion_parameters( + topology, new_system, charge_difference=1, forcefield=forcefield, + ) + + # Should still get a +1 charge from the forcefield fallback + assert ion_charge.value_in_unit(omm_unit.elementary_charge) == pytest.approx(1.0) + + +def test_get_ion_parameters_no_ions_at_all(benzene_modifications): + """ + Scenario 3: no ions present at all (e.g. zero ionic strength) — should + fall back to the forcefield. + """ + smc = benzene_modifications["benzene"] + offmol = smc.to_openff() + settings = openmm_rfe.RelativeHybridTopologyProtocol.default_settings() + + system_generator = system_creation.get_system_generator( + forcefield_settings=settings.forcefield_settings, + integrator_settings=settings.integrator_settings, + thermo_settings=settings.thermo_settings, + cache=None, + has_solvent=True, + ) + system_generator.create_system( + offmol.to_topology().to_openmm(), + molecules=[offmol], + ) + + # Solvate with zero ionic strength — no ions added. + modeller, _ = system_creation.get_omm_modeller( + protein_comp=None, + solvent_comp=openfe.SolventComponent( + ion_concentration=0 * unit.molar, + ), + small_mols={smc: offmol}, + omm_forcefield=system_generator.forcefield, + solvent_settings=settings.solvation_settings, + ) + + topology = modeller.getTopology() + system = system_generator.create_system(topology, molecules=[offmol]) + + with pytest.warns(UserWarning, match="No monovalent ion"): + ion_charge, ion_sigma, ion_epsilon = topologyhelpers._get_ion_parameters( + topology, system, + charge_difference=1, + forcefield=system_generator.forcefield, ) + assert ion_charge.value_in_unit(omm_unit.elementary_charge) == pytest.approx(1.0) + def test_get_alchemical_waters_no_waters( benzene_solvent_openmm_system, ): - system, topology, positions = benzene_solvent_openmm_system + system, topology, positions, forcefield = benzene_solvent_openmm_system errmsg = "There are no waters" @@ -1969,7 +2065,7 @@ def test_handle_alchemwats_incorrect_count( """ Check that an error is thrown when charge_difference != len(water_resids) """ - system, topology, positions = benzene_solvent_openmm_system + system, topology, positions, forcefield = benzene_solvent_openmm_system errmsg = "There should be as many alchemical water residues:" @@ -1980,7 +2076,8 @@ def test_handle_alchemwats_incorrect_count( system=system, system_mapping={}, charge_difference=1, - solvent_component=openfe.SolventComponent(), + forcefield=forcefield, + water_resname="HOH", ) @@ -1990,7 +2087,7 @@ def test_handle_alchemwats_too_many_nbf( """ Check that an error is thrown when there are multiple NonbondedForces """ - system, topology, positions = benzene_solvent_openmm_system + system, topology, positions, forcefield = benzene_solvent_openmm_system new_system = copy.deepcopy(system) new_system.addForce(NonbondedForce()) @@ -2004,7 +2101,8 @@ def test_handle_alchemwats_too_many_nbf( system=new_system, system_mapping={}, charge_difference=1, - solvent_component=openfe.SolventComponent(), + forcefield=forcefield, + water_resname="HOH", ) @@ -2015,7 +2113,7 @@ def test_handle_alchemwats_vsite_water( Check that an error is thrown when trying to use a 4 site water as an alchemical species """ - system, topology, positions = benzene_tip4p_solvent_openmm_system + system, topology, positions, forcefield = benzene_tip4p_solvent_openmm_system errmsg = "Non 3-site waters" @@ -2026,7 +2124,8 @@ def test_handle_alchemwats_vsite_water( system=system, system_mapping={}, charge_difference=1, - solvent_component=openfe.SolventComponent(), + forcefield=forcefield, + water_resname="HOH", ) @@ -2037,7 +2136,7 @@ def test_handle_alchemwats_incorrect_atom( """ Check that an error is thrown when charge_difference != len(water_resids) """ - system, topology, positions = benzene_solvent_openmm_system + system, topology, positions, forcefield = benzene_solvent_openmm_system # modify the charge of hydrogen atom 25 new_system = copy.deepcopy(system) # protect the session scoped object @@ -2054,7 +2153,8 @@ def test_handle_alchemwats_incorrect_atom( system=new_system, system_mapping=benzene_self_system_mapping, charge_difference=1, - solvent_component=openfe.SolventComponent(), + forcefield=forcefield, + water_resname="HOH", ) @@ -2062,7 +2162,7 @@ def test_handle_alchemical_wats( benzene_solvent_openmm_system, benzene_self_system_mapping, ): - system, topology, positions = benzene_solvent_openmm_system + system, topology, positions, forcefield = benzene_solvent_openmm_system n_env = len(benzene_self_system_mapping["old_to_new_env_atom_map"]) n_core = len(benzene_self_system_mapping["old_to_new_core_atom_map"]) @@ -2073,7 +2173,8 @@ def test_handle_alchemical_wats( system=system, system_mapping=benzene_self_system_mapping, charge_difference=1, - solvent_component=openfe.SolventComponent(), + forcefield=forcefield, + water_resname="HOH", ) # check the mappings @@ -2088,9 +2189,9 @@ def test_handle_alchemical_wats( # system parameters checks nbf = [i for i in system.getForces() if isinstance(i, NonbondedForce)][0] - # check the oxygen parameters - i_chg, i_sig, i_eps, o_chg, h_chg = topologyhelpers._get_ion_and_water_parameters( - topology, system, "NA", "HOH" + # Verify the oxygen was mutated to ion parameters + i_chg, i_sig, i_eps = topologyhelpers._get_ion_parameters( + topology, system, charge_difference=1, forcefield=forcefield, ) charge, sigma, epsilon = nbf.getParticleParameters(24) diff --git a/src/openfe/tests/protocols/openmm_rfe/test_hybrid_top_validation.py b/src/openfe/tests/protocols/openmm_rfe/test_hybrid_top_validation.py index 6d5db1249..c29b1b37f 100644 --- a/src/openfe/tests/protocols/openmm_rfe/test_hybrid_top_validation.py +++ b/src/openfe/tests/protocols/openmm_rfe/test_hybrid_top_validation.py @@ -441,7 +441,7 @@ def test_get_charge_difference(mapping_name, result, request, caplog): mapping = request.getfixturevalue(mapping_name) caplog.set_level(logging.INFO) - ion = r"Na+" if result == -1 else r"Cl-" + ion = r"NA" if result == -1 else r"CL" msg = ( f"A charge difference of {result} is observed " "between the end states. This will be addressed by "