OpenFreeEnergy / openfe

The Open Free Energy toolkit
https://docs.openfree.energy
MIT License
140 stars 20 forks source link

CI fails with OpenMM 8.2 in HTF.hyrid_system to mdtraj.topology transformation #1016

Open IAlibay opened 1 day ago

IAlibay commented 1 day ago

It looks like the bond order is being passed as a Single rather than an int which is causing a comparison failure:

Example:

benzene_complex_system = ChemicalSystem(name=, components={'ligand': SmallMoleculeComponent(name=benzene), 'solvent': SolventComponent(name=O, Na+, Cl-), 'protein': ProteinComponent(name=T4_protein)})
toluene_complex_system = ChemicalSystem(name=, components={'ligand': SmallMoleculeComponent(name=toluene), 'solvent': SolventComponent(name=O, Na+, Cl-), 'protein': ProteinComponent(name=T4_protein)})
benzene_to_toluene_mapping = LigandAtomMapping(componentA=SmallMoleculeComponent(name=benzene), componentB=SmallMoleculeComponent(name=toluene), componentA_to_componentB={0: 4, 1: 5, 2: 6, 3: 7, 4: 8, 5: 9, 6: 10, 7: 11, 8: 12, 9: 13, 11: 14}, annotations={})
method = 'repex'
tmpdir = local('/tmp/pytest-of-runner/pytest-0/popen-gw1/test_dry_run_complex_repex_0')

    @pytest.mark.slow
    @pytest.mark.parametrize('method', ['repex', 'sams', 'independent'])
    def test_dry_run_complex(benzene_complex_system, toluene_complex_system,
                             benzene_to_toluene_mapping, method, tmpdir):
        # this will be very time consuming
        settings = openmm_rfe.RelativeHybridTopologyProtocol.default_settings()
        settings.simulation_settings.sampler_method = method
        settings.protocol_repeats = 1
        settings.output_settings.output_indices = 'protein or resname  UNK'

        protocol = openmm_rfe.RelativeHybridTopologyProtocol(
                settings=settings,
        )
        dag = protocol.create(
            stateA=benzene_complex_system,
            stateB=toluene_complex_system,
            mapping=benzene_to_toluene_mapping,
        )
        dag_unit = list(dag.protocol_units)[0]

        with tmpdir.as_cwd():
>           sampler = dag_unit.run(dry=True)['debug']['sampler']

openfe/tests/protocols/test_openmm_equil_rfe_protocols.py:906: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
openfe/protocols/openmm_rfe/equil_rfe_methods.py:870: in run
    hybrid_factory = _rfe_utils.relative.HybridTopologyFactory(
openfe/protocols/openmm_rfe/_rfe_utils/relative.py:224: in __init__
    self._hybrid_topology = self._create_mdtraj_topology()
openfe/protocols/openmm_rfe/_rfe_utils/relative.py:2248: in _create_mdtraj_topology
    old_top = mdt.Topology.from_openmm(self._old_topology)
../../../micromamba/envs/openfe_env/lib/python3.10/site-packages/mdtraj/core/topology.py:460: in from_openmm
    out.add_bond(
../../../micromamba/envs/openfe_env/lib/python3.10/site-packages/mdtraj/core/topology.py:866: in add_bond
    self._bonds.append(Bond(atom1, atom2, type=type, order=order))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

cls = <class 'mdtraj.core.topology.Bond'>, atom1 = ACE0-C, atom2 = ACE0-CH3
type = None, order = Single

    def __new__(cls, atom1, atom2, type=None, order=None):
        """Construct a new Bond.  You should call add_bond()
        on the Topology instead of calling this directly.

        Must use __new__ constructor since this is an immutable class
        """
        bond = super().__new__(cls, atom1, atom2)
        assert isinstance(type, Singleton) or type is None, "Type must be None or a Singleton"
>       assert order is None or 1 <= order <= 3, "Order must be int between 1 to 3 or None"
E       TypeError: '<=' not supported between instances of 'int' and 'Single'

../../../micromamba/envs/openfe_env/lib/python3.10/site-packages/mdtraj/core/topology.py:1815: TypeError
IAlibay commented 1 day ago

@mikemhenry : I ran TYK2

mikemhenry commented 1 day ago

It looks like the change in behavior is that mdtraj is expecting None or Singleton and now openmm is using int

EDIT: Okay this isn't in our sprint so I need to stop looking at this, but the type is actually passed into the constructor -- so we need to see where we are calling this/why the type is an int, like here:

openfe/protocols/openmm_rfe/_rfe_utils/relative.py:2248: in _create_mdtraj_topology
    old_top = mdt.Topology.from_openmm(self._old_topology)