API: Make Interchange.topology a required field

openff/interchange/_tests/unit_tests/components/test_interchange.py

@@ -88,7 +88,7 @@ def test_missing_electrostatics_handler(self, tip3p, water):
         assert out["Electrostatics"].cutoff == 7.89 * unit.angstrom
 
     def test_box_setter(self):
-        tmp = Interchange()
+        tmp = Interchange(topology=Molecule.from_smiles("O").to_topology())
 
         with pytest.raises(ValidationError):
             tmp.box = [2, 2, 3, 90, 90, 90]
@@ -156,10 +156,10 @@ def test_from_sage(self, sage):
     def test_validate_simple_topology(self, sage):
         from openff.interchange.components.toolkit import _simple_topology_from_openmm
 
-        tmp = Interchange()
-        tmp.topology = _simple_topology_from_openmm(
+        topology = _simple_topology_from_openmm(
             Molecule.from_smiles("CCO").to_topology().to_openmm(),
         )
+        Interchange(topology=topology)
 
     def test_from_sage_molecule_list(self, sage):
         out = Interchange.from_smirnoff(

openff/interchange/_tests/unit_tests/interop/openmm/_import/test_import.py

@@ -75,13 +75,19 @@ def test_different_ways_to_process_box_vectors(
     ):
         monkeypatch.setenv("INTERCHANGE_EXPERIMENTAL", "1")
 
+        topology = Molecule.from_smiles("C").to_topology()
+
         if as_argument:
             box = Interchange.from_openmm(
                 system=simple_system,
+                topology=topology,
                 box_vectors=simple_system.getDefaultPeriodicBoxVectors(),
             ).box
         else:
-            box = Interchange.from_openmm(system=simple_system).box
+            box = Interchange.from_openmm(
+                system=simple_system,
+                topology=topology,
+            ).box
 
         assert box.shape == (3, 3)
 

openff/interchange/_tests/unit_tests/operations/test_combine.py

@@ -29,7 +29,7 @@ def test_basic_combination(self, monkeypatch, sage_unconstrained):
         interchange.positions = mol.conformers[0]
 
         # Copy and translate atoms by [1, 1, 1]
-        other = Interchange()
+        other = Interchange(topology=copy.deepcopy(interchange.topology))
         other = copy.deepcopy(interchange)
         other.positions += 1.0 * unit.nanometer
 

openff/interchange/components/interchange.py

@@ -59,7 +59,7 @@ class Interchange(_BaseModel):
     """
 
     collections: _AnnotatedCollections = Field(dict())
-    topology: _AnnotatedTopology | None = Field(None)
+    topology: _AnnotatedTopology
     mdconfig: MDConfig | None = Field(None)
     box: _BoxQuantity | None = Field(None)  # Needs shape/OpenMM validation
     positions: _PositionsQuantity | None = Field(None)  # Ditto

openff/interchange/foyer/_create.py

@@ -45,20 +45,19 @@ def _create_interchange(
     box: Quantity | None = None,
     positions: Quantity | None = None,
 ) -> Interchange:
-    interchange = Interchange()
-    _topology = Topology(topology)
+    interchange = Interchange(topology=Topology(topology))
 
-    interchange.positions = _infer_positions(_topology, positions)
+    interchange.positions = _infer_positions(interchange.topology, positions)
 
-    interchange.box = _topology.box_vectors if box is None else box
+    interchange.box = interchange.topology.box_vectors if box is None else box
 
     # This block is from a mega merge, unclear if it's still needed
     for name, handler_class in get_handlers_callable().items():
         interchange.collections[name] = handler_class()
 
     vdw_handler = interchange["vdW"]
     vdw_handler.scale_14 = force_field.lj14scale
-    vdw_handler.store_matches(force_field, topology=_topology)
+    vdw_handler.store_matches(force_field, topology=interchange.topology)
     vdw_handler.store_potentials(force_field=force_field)
 
     atom_slots = vdw_handler.key_map
@@ -72,16 +71,18 @@ def _create_interchange(
 
     for name, handler in interchange.collections.items():
         if name not in ["vdW", "Electrostatics"]:
-            handler.store_matches(atom_slots, topology=_topology)
+            handler.store_matches(atom_slots, topology=interchange.topology)
             handler.store_potentials(force_field)
 
     # TODO: Populate .mdconfig, but only after a reasonable number of state mutations have been tested
 
     charges = electrostatics.charges
 
-    for molecule in _topology.molecules:
+    for molecule in interchange.topology.molecules:
         molecule_charges = [
-            charges[TopologyKey(atom_indices=(_topology.atom_index(atom),))].m
+            charges[
+                TopologyKey(atom_indices=(interchange.topology.atom_index(atom),))
+            ].m
             for atom in molecule.atoms
         ]
         molecule.partial_charges = Quantity(
@@ -92,6 +93,4 @@ def _create_interchange(
     interchange.collections["vdW"] = vdw_handler
     interchange.collections["Electrostatics"] = electrostatics
 
-    interchange.topology = _topology
-
     return interchange

openff/interchange/interop/gromacs/_interchange.py

@@ -292,7 +292,7 @@ def to_interchange(
 
             molecule_start_index += len(molecule_type.atoms)
 
-    interchange = Interchange()
+    interchange = Interchange(topology=_convert_topology(system))
 
     interchange.collections.update(
         {
@@ -307,7 +307,6 @@ def to_interchange(
         },
     )
 
-    interchange.topology = _convert_topology(system)
     interchange.positions = system.positions
     interchange.box = system.box
 

openff/interchange/interop/openmm/_import/_import.py

@@ -37,15 +37,41 @@
 def from_openmm(
     *,
     system: "openmm.System",
+    topology: Union["openmm.app.Topology", Topology],
     positions: Quantity | None = None,
-    topology: Union["openmm.app.Topology", Topology, None] = None,
     box_vectors: Quantity | None = None,
 ) -> "Interchange":
     """Create an Interchange object from OpenMM data."""
     from openff.interchange import Interchange
 
     _check_compatible_inputs(system=system, topology=topology)
-    interchange = Interchange()
+
+    if isinstance(topology, openmm.app.Topology):
+        from openff.units.openmm import from_openmm as from_openmm_
+
+        from openff.interchange.components.toolkit import _simple_topology_from_openmm
+
+        openff_topology = _simple_topology_from_openmm(topology)
+
+        if topology.getPeriodicBoxVectors() is not None:
+            openff_topology.box_vectors = from_openmm_(topology.getPeriodicBoxVectors())
+
+        # OpenMM topologies do not store positions
+
+    elif isinstance(topology, Topology):
+
+        openff_topology = topology
+        positions = openff_topology.get_positions()
+
+    elif topology is None:
+        raise ValueError("A topology must be provided.")
+
+    else:
+        raise ValueError(
+            f"Could not process `topology` argument of type {type(topology)=}.",
+        )
+
+    interchange = Interchange(topology=openff_topology)
 
     if system:
         constraints = _convert_constraints(system)
@@ -74,22 +100,6 @@ def from_openmm(
                     f"Unsupported OpenMM Force type ({type(force)}) found.",
                 )
 
-    if isinstance(topology, openmm.app.Topology):
-        from openff.interchange.components.toolkit import _simple_topology_from_openmm
-
-        openff_topology = _simple_topology_from_openmm(topology)
-
-        interchange.topology = openff_topology
-
-    elif isinstance(topology, Topology):
-
-        interchange.topology = topology
-        interchange.positions = topology.get_positions()
-
-    elif topology is None:
-
-        interchange.topology = topology
-
     if positions is None:
 
         warnings.warn(
@@ -104,29 +114,25 @@ def from_openmm(
         interchange.positions = positions
 
     if box_vectors is not None:
-        _box_vectors: Quantity = box_vectors
-
-    elif topology is not None:
-        if isinstance(topology, openmm.app.Topology):
-            from openff.units.openmm import from_openmm as from_openmm_
+        _box_vectors = box_vectors
 
-            _box_vectors = from_openmm_(topology.getPeriodicBoxVectors())
-        elif isinstance(topology, Topology):
-            _box_vectors = topology.box_vectors
+    elif interchange.topology.box_vectors is not None:
+        _box_vectors = interchange.topology.box_vectors
 
     else:
+        # If there is no box argument passed and the topology is non-periodic
+        # and the system does not have default box vectors, it'll end up as None
         from openff.units.openmm import from_openmm as from_openmm_
 
         _box_vectors = from_openmm_(system.getDefaultPeriodicBoxVectors())
 
-    # TODO: There should probably be a more public box validator, checking for shape, etc.
+    # TODO: Does this run through the Interchange.box validator?
     interchange.box = _box_vectors
 
-    if interchange.topology is not None:
-        if interchange.topology.n_bonds > len(interchange.collections["Bonds"].key_map):
-            # There are probably missing (physics) bonds from rigid waters. The topological
-            # bonds are probably processed correctly.
-            _fill_in_rigid_water_bonds(interchange)
+    if interchange.topology.n_bonds > len(interchange.collections["Bonds"].key_map):
+        # There are probably missing (physics) bonds from rigid waters. The topological
+        # bonds are probably processed correctly.
+        _fill_in_rigid_water_bonds(interchange)
 
     return interchange
 

openff/interchange/smirnoff/_create.py

@@ -107,41 +107,50 @@ def _create_interchange(
 
     _check_supported_handlers(force_field)
 
-    interchange = Interchange()
+    # interchange = Interchange(topology=topology)
+    # or maybe
+    interchange = Interchange(topology=validate_topology(topology))
 
-    # TODO: Need to re-introduce logic lost when validator re-use was nuked
-    _topology = validate_topology(topology)
+    interchange.positions = _infer_positions(interchange.topology, positions)
 
-    interchange.positions = _infer_positions(_topology, positions)
+    interchange.box = interchange.topology.box_vectors if box is None else box
 
-    interchange.box = _topology.box_vectors if box is None else box
-
-    _bonds(interchange, force_field, _topology, partial_bond_orders_from_molecules)
+    _bonds(
+        interchange,
+        force_field,
+        interchange.topology,
+        partial_bond_orders_from_molecules,
+    )
     _constraints(
         interchange,
         force_field,
-        _topology,
+        interchange.topology,
         bonds=interchange.collections.get("Bonds", None),  # type: ignore[arg-type]
     )
-    _angles(interchange, force_field, _topology)
-    _propers(interchange, force_field, _topology, partial_bond_orders_from_molecules)
-    _impropers(interchange, force_field, _topology)
+    _angles(interchange, force_field, interchange.topology)
+    _propers(
+        interchange,
+        force_field,
+        interchange.topology,
+        partial_bond_orders_from_molecules,
+    )
+    _impropers(interchange, force_field, interchange.topology)
 
-    _vdw(interchange, force_field, _topology)
+    _vdw(interchange, force_field, interchange.topology)
     _electrostatics(
         interchange,
         force_field,
-        _topology,
+        interchange.topology,
         charge_from_molecules,
         allow_nonintegral_charges,
     )
-    _plugins(interchange, force_field, _topology)
+    _plugins(interchange, force_field, interchange.topology)
 
-    _virtual_sites(interchange, force_field, _topology)
+    _virtual_sites(interchange, force_field, interchange.topology)
 
-    _gbsa(interchange, force_field, _topology)
+    _gbsa(interchange, force_field, interchange.topology)
 
-    interchange.topology = _topology
+    interchange.topology = interchange.topology
 
     return interchange