diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 7aaf28b24..c5b92c41a 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -28,18 +28,9 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest] - python-version: ["3.9", "3.10", "3.11", "3.12"] + python-version: ["3.12"] rdkit: [true, false] - openeye: [true, false] - exclude: - - rdkit: false - openeye: false - - openeye: true - python-version: "3.10" - - openeye: true - python-version: "3.11" - - openeye: true - python-version: "3.12" + openeye: [false] env: OE_LICENSE: ${{ github.workspace }}/oe_license.txt diff --git a/devtools/conda-envs/openeye-examples.yaml b/devtools/conda-envs/openeye-examples.yaml index df1fa228b..b0b3b301b 100644 --- a/devtools/conda-envs/openeye-examples.yaml +++ b/devtools/conda-envs/openeye-examples.yaml @@ -14,12 +14,12 @@ dependencies: - xmltodict - python-constraint - openmm >=7.6 - - openff-forcefields >=2023.05.1 + - openff-forcefields >=2023.11 - smirnoff99Frosst - openff-amber-ff-ports >=0.0.3 - openff-units =0.2.0 - openff-utilities >=0.1.5 - - openff-interchange-base ==0.3.14 + - openff-interchange-base ==0.3.17 - openff-nagl ==0.3.2 - typing_extensions # Toolkit-specific diff --git a/devtools/conda-envs/rdkit-examples.yaml b/devtools/conda-envs/rdkit-examples.yaml index 84c3a5e64..91faae930 100644 --- a/devtools/conda-envs/rdkit-examples.yaml +++ b/devtools/conda-envs/rdkit-examples.yaml @@ -13,12 +13,12 @@ dependencies: - xmltodict - python-constraint - openmm >=7.6 - - openff-forcefields >=2023.05.1 + - openff-forcefields >=2023.11 - smirnoff99Frosst - openff-amber-ff-ports >=0.0.3 - openff-units =0.2.0 - openff-utilities >=0.1.5 - - openff-interchange-base ==0.3.14 + - openff-interchange-base ==0.3.17 - openff-nagl ==0.3.2 - typing_extensions # Toolkit-specific diff --git a/devtools/conda-envs/rdkit.yaml b/devtools/conda-envs/rdkit.yaml index d76459648..f7a6e6a74 100644 --- a/devtools/conda-envs/rdkit.yaml +++ b/devtools/conda-envs/rdkit.yaml @@ -20,6 +20,7 @@ dependencies: - openff-amber-ff-ports - openff-utilities >=0.1.5 - openff-interchange-base >=0.3.17 + - openff-nagl-base ==0.3.2 - typing_extensions # Toolkit-specific - ambertools >=22 diff --git a/docs/releasehistory.md b/docs/releasehistory.md index 248b5b1f1..27832b913 100644 --- a/docs/releasehistory.md +++ b/docs/releasehistory.md @@ -15,6 +15,7 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w ### Bugfixes - [PR #1740](https://github.com/openforcefield/openff-toolkit/pull/1740): Updates for Mypy 1.6. +- [PR #1756](https://github.com/openforcefield/openff-toolkit/pull/1756): Fixes issue [#1739](https://github.com/openforcefield/openff-toolkit/issues/1739), where virtual sites would be double-created under some circumstances. ### New features diff --git a/docs/users/virtualsites.md b/docs/users/virtualsites.md index e46c69ae8..292c1c53a 100644 --- a/docs/users/virtualsites.md +++ b/docs/users/virtualsites.md @@ -118,8 +118,8 @@ site parameters. Let us consider 4-, 5-, and 6-point water models: ## Ordering of atoms and virtual sites -The toolkit handles the orders the atoms and virtual sites in a topology in a -specific manner for internal convenience. +The OpenFF Toolkit and Interchange currently add all new virtual particles to the "end" of a Topology, +such that the particle indices of all newly-created virtual particles are higher than index of the last atom. In addition, due to the fact that a virtual site may contain multiple particles coupled to single parameters, the toolkit makes a distinction between a virtual *site*, and a virtual diff --git a/examples/virtual_sites/vsite_showcase.ipynb b/examples/virtual_sites/vsite_showcase.ipynb index 20a2b356a..4937809b7 100644 --- a/examples/virtual_sites/vsite_showcase.ipynb +++ b/examples/virtual_sites/vsite_showcase.ipynb @@ -19,7 +19,7 @@ { "data": { "application/vnd.jupyter.widget-view+json": { - "model_id": "739f5555d95d467fbfc7337d30b71b64", + "model_id": "ba621889dffa475e80a6a6b5f8f476e5", "version_major": 2, "version_minor": 0 }, @@ -32,12 +32,12 @@ "source": [ "import time\n", "\n", - "import numpy as np\n", + "import numpy\n", "import openmm\n", "import openmm.app\n", "import openmm.unit\n", "from openff.interchange import Interchange\n", - "from openff.units import unit\n", + "from openff.units import Quantity, unit\n", "\n", "from openff.toolkit import ForceField, Molecule, Topology" ] @@ -175,70 +175,70 @@ "\n", "\n", " \n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", - "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", "" ], "text/plain": [ @@ -314,7 +314,7 @@ "output_type": "stream", "text": [ "Starting simulation\n", - "Elapsed time 1.08 seconds\n", + "Elapsed time 0.96 seconds\n", "Done!\n" ] } @@ -335,7 +335,7 @@ { "data": { "application/vnd.jupyter.widget-view+json": { - "model_id": "333d9ab68b7c4198944f6f9c3bc56517", + "model_id": "c05515b6f75647ed89ebf066d9f99c48", "version_major": 2, "version_minor": 0 }, @@ -375,45 +375,24 @@ } }, "source": [ - "Parameterizes a water box with OpenFF and OpenMM force fields. Currently set up\n", - "to use a TIP5P definition. The code examines the geometry and energy between the\n", - "two, and examines cases where minimization is performed. Specifically, the code\n", - "compares the four possible combinations:\n", - " - oFF and oMM geometry/energy, minimized separately and then compared\n", - " - oFF and oMM geometry/energy using no minimization\n", - " - Geometry minimized using oFF, then a single point is done with oMM\n", - " - Geometry minimized using oMM, then a single point is done with oFF\n", - "\n", - "The virtual site definitions give differences in geometry and energy mostly due\n", - "to how they were defined from their parent atoms. OpenMM uses an OutOfPlaneSite\n", - "definition, whereas OpenFF uses the LocalCoordinatesSite definition (both are\n", - "OpenMM types). In the OutOfPlaneSite definition, both angle and distance are\n", - "variable since the defined out-of-plane angle depends on a weighted vector cross.\n", - "The cross is a function of the O-H vectors, so the virtual sites are sensitive\n", - "to the molecular geometry. In the OpenFF version, the distance is fixed to a constant\n", - "value, and the out-of-plane angle is explicitly required in the OpenFF spec.\n", - "\n", - "In this example, the OpenFF parameter definition (the \"offxml\") is a string\n", - "further below in the `main` function, and can be easily modified to explore\n", - "force field parameterization. The OpenMM definition is loaded from its internal\n", - "default location, and acts as a reference. One can change this this to a different\n", - "filename to compare other force fields.\n", - "\n", - "This example is somewhat hardcoded to operate on water molecules, but can be easily\n", - "modified to examine other cases as well. The only major assumption is that the\n", - "system is homogenous, i.e., all of the molecules are same. The reason this is\n", - "assumed is mostly due to the difference in how virtual sites are handled between\n", - "OpenFF and OpenMM. Whereas OpenMM interleaves the virtual site particles between\n", - "the atomic particles, OpenFF instead aggregates all virtual sites and places them\n", - "last. The code below does assume that, barring this difference, the virtual sites\n", - "are added in the same order.\n", - "\n", - "The example begins in `run_tests` by defining a grid of water molecules, with\n", - "the default being a single water molecule (Nx=1, Ny=1, Nz=1). From this, the\n", - "calculations described in the first paragraph above are performed. The energy\n", - "difference and distance between the two geometries, per atom, is then reported.\n", - "There are commented lines that print the entire set of coordinates, and can be\n", - "uncommented if desired." + "Parameterizes a water box with OpenFF/SMIRNOFF and OpenMM implementations of TIP5P.\n", + "\n", + "This code examines the geometry and energy between the two, and examines cases where minimization is performed.\n", + "\n", + "The virtual site definitions give rise to small differences in geometry in due to to how they are defined relative to their parent atoms.\n", + "OpenMM's force field produces `openmm.OutOfPlaneSite`s whereas OpenFF's force field produces in `openmm.LocalCoordinatesSite`s.\n", + "(In each case, these objects are OpenMM objects to be used in OpenMM simulations; implementations in other engines are not covered here.)\n", + "`OutOfPlaneSite` virtual sites' positions are are determined based on the positions of the orientation atoms.\n", + "(In water models, the \"parent\" atom is oxygen and the \"orientation\" atoms are the oxygen and hydrogens; in other chemistries virtual sites may be definited differently.)\n", + "`LocalCoordinatesSite` virtual sites' positions relative to orientation atoms are fixed to a constant value in order to adhere to the SMIRNOFF specification.\n", + "A consequence of this difference is that tiny fluctuations in the molecular geometry cause slightly different coordinates and, therefore, energy evaluations.\n", + "This can arise from marginally different initial positions or from the dynamics of a simulation, but the descriptions at equilibrium should match.\n", + "Further nuance is described in the [OpenMM User Guide](http://docs.openmm.org/8.0.0/userguide/theory/05_other_features.html#virtual-sites) and API docs.\n", + "\n", + "This example uses water molecules, but both OpenFF and OpenMM infrastructure support a variety of other virtual site definitions for use in other chemistries.\n", + "\n", + "One key difference between OpenFF and OpenMM code paths is the bookkeepping of virtual sites relative to atomistic particles.\n", + "OpenMM interleaves the virtual site particles at the end of _each molecule_ whereas OpenFF places all all virtual sites at the end of the `openmm.System`, topology, and related objects." ] }, { @@ -422,18 +401,18 @@ "metadata": {}, "outputs": [], "source": [ - "def _collate_virtual_site_positions(atom_positions: np.ndarray) -> np.ndarray:\n", + "def _collate_virtual_site_positions(atom_positions: numpy.ndarray) -> numpy.ndarray:\n", " \"\"\"Given an array of atomic positions of water, collate virtual particles between molecules.\"\"\"\n", - " padded_positions = np.zeros(shape=(2, 3))\n", + " padded_positions = numpy.zeros(shape=(2, 3))\n", " num_atoms_per_mol = 3\n", "\n", " def mol_positions(i, atom_positions):\n", " this_mol_atom_coordinates = atom_positions[\n", " i * num_atoms_per_mol : (i + 1) * num_atoms_per_mol\n", " ]\n", - " return np.vstack([this_mol_atom_coordinates, padded_positions])\n", + " return numpy.vstack([this_mol_atom_coordinates, padded_positions])\n", "\n", - " return np.vstack([mol_positions(i, atom_positions) for i in range(2)])" + " return numpy.vstack([mol_positions(i, atom_positions) for i in range(2)])" ] }, { @@ -452,7 +431,7 @@ " openmm_system: openmm.app.Simulation,\n", " particle_positions: openmm.unit.Quantity,\n", " minimize=False,\n", - ") -> tuple[np.ndarray, unit.Quantity]:\n", + ") -> tuple[numpy.ndarray, Quantity]:\n", " \"\"\"\n", " Calculate particle positions and potential energy of the a system.\n", "\n", @@ -460,7 +439,7 @@ " ----------\n", " openmm_topology: openmm.app.Topology, OpenMM Topology\n", " openmm_system: openmm.app.System, OpenMM System\n", - " particle_positions: openff.units.unit.Quantity, (N, 3) array of positions in nanometers\n", + " particle_positions: openff.units.Quantity, (N, 3) array of positions in nanometers\n", " minimize: bool, Whether or not to perform an energy minimization before calculating energy\n", "\n", " Returns\n", @@ -523,8 +502,10 @@ " -------\n", " water_box: list[Molecule], A list of Molecule objcets with a 3D conformation\n", " \"\"\"\n", + " from math import cos, sin\n", + "\n", " Lx, Ly, Lz = (num_duplicates[i] * spacing[i] for i in range(3))\n", - " Z, Y, X = np.mgrid[\n", + " Z, Y, X = numpy.mgrid[\n", " 0 : Lz : spacing[0],\n", " 0 : Ly : spacing[1],\n", " 0 : Lx : spacing[2],\n", @@ -536,16 +517,47 @@ " water_reference.atoms[0].name = \"O\"\n", " water_reference.atoms[1].name = \"H1\"\n", " water_reference.atoms[2].name = \"H2\"\n", + "\n", " # Add ideal TIP5P geometry\n", + " bond_length = Quantity(0.9572, unit.angstrom)\n", + " theta = Quantity(104.52, unit.degree).to(unit.radian)\n", + "\n", " water_reference.add_conformer(\n", - " [[0.0, 0.0, 0.0], [-0.7815, 0.5526, 0.0], [0.7815, 0.5526, 0.0]] * unit.angstrom\n", + " bond_length\n", + " * Quantity(\n", + " [\n", + " [0.0, 0.0, 0.0],\n", + " [-sin(theta / 2), cos(theta / 2), 0.0],\n", + " [sin(theta / 2), cos(theta / 2), 0.0],\n", + " ]\n", + " )\n", " )\n", "\n", " for i, xyz in enumerate(XYZ):\n", " water_box[i] = Molecule(water_reference)\n", " water_box[i].conformers[0] = water_box[i].conformers[0] + xyz * unit.angstrom\n", "\n", - " return water_box" + " return water_box\n", + "\n", + "\n", + "def reorder(\n", + " number_molecules: int,\n", + " atoms_per_molecule: int = 3,\n", + " virtual_sites_per_molecule: int = 2,\n", + ") -> list[int]:\n", + " \"\"\"Return an mapping between collated and un-collated particle indices.\"\"\"\n", + " particles_per_molecule = atoms_per_molecule + virtual_sites_per_molecule\n", + "\n", + " atoms = list()\n", + " virtual_sites = list()\n", + "\n", + " for particle_index in range(number_molecules * particles_per_molecule):\n", + " if particle_index % particles_per_molecule < atoms_per_molecule:\n", + " atoms.append(particle_index)\n", + " else:\n", + " virtual_sites.append(particle_index)\n", + "\n", + " return atoms + virtual_sites" ] }, { @@ -582,7 +594,7 @@ " # First, get an OpenMM Topology and atom positions with no virtual sites\n", " _topology: openmm.app.Topology = Topology.from_molecules(water).to_openmm()\n", "\n", - " atom_positions_unitless = np.vstack(\n", + " atom_positions_unitless = numpy.vstack(\n", " [mol.conformers[0].m_as(unit.nanometer) for mol in water]\n", " )\n", " atom_positions = openmm.unit.Quantity(\n", @@ -662,66 +674,27 @@ }, "outputs": [], "source": [ - "def print_info(xyz, ene, name, crd_units=unit.angstrom) -> str:\n", - " print(f\"Results for: {name}\\nEnergy: {ene}\\nCoordinates:{xyz * crd_units}\\n\")" - ] - }, - { - "cell_type": "code", - "execution_count": 14, - "metadata": { - "pycharm": { - "name": "#%%\n" - } - }, - "outputs": [], - "source": [ - "tip5p_offxml = \"\"\"\n", - "\n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - " \n", - "\n", - "\"\"\"\n", + "def print_info(\n", + " coordinate_difference: Quantity,\n", + " energy_difference: openmm.unit.Quantity,\n", + " name: str,\n", + "):\n", + " from openff.units.openmm import from_openmm\n", "\n", + " _energy_difference = from_openmm(energy_difference)\n", "\n", - "constraints = \"\"\"\n", - " \n", - " \n", - " \n", - " \n", - "\"\"\"" + " print(\n", + " f\"Results for: {name}\\n\"\n", + " f\"Energy difference ({_energy_difference.units}):\\n\\t\"\n", + " f\"{_energy_difference.m:0.3e}\\n\"\n", + " f\"Coordinates difference ({coordinate_difference.units}), norm):\\n\\t\"\n", + " f\"{coordinate_difference.m:0.3e}\\n\"\n", + " )" ] }, { "cell_type": "code", - "execution_count": 15, + "execution_count": 14, "metadata": { "pycharm": { "name": "#%%\n" @@ -729,16 +702,16 @@ }, "outputs": [], "source": [ - "# The TIP5P force field in SMIRNOFF format\n", - "openff_force_field = ForceField(tip5p_offxml)\n", + "# The TIP5P force field in SMIRNOFF format, provided by the OpenFF Toolkit\n", + "openff_force_field = ForceField(\"tip5p.offxml\")\n", "\n", - "# The standard OpenMM definition of tip5p\n", + "# The OpenMM definition of TIP5P\n", "openmm_force_field = openmm.app.ForceField(\"tip5p.xml\")" ] }, { "cell_type": "code", - "execution_count": 16, + "execution_count": 15, "metadata": { "pycharm": { "name": "#%%\n" @@ -750,9 +723,11 @@ "name": "stdout", "output_type": "stream", "text": [ - "Results for: OpenFF - OpenMM norm\n", - "Energy: 0.6343460083007812 kJ/mol\n", - "Coordinates:[0.0 0.0 0.0 0.027415848854748962 0.027415848854748962 0.0 0.0 0.0 0.02741584885474892 0.02741584885474892] angstrom\n", + "Results for: OpenFF - OpenMM comparison (per molecule)\n", + "Energy difference (kilojoule / mole):\n", + "\t1.907e-06\n", + "Coordinates difference (angstrom), norm):\n", + "\t1.624e-07\n", "\n" ] } @@ -763,36 +738,41 @@ "num_duplicates = (2, 1, 1) # 2x2x2 = 8 water molecules\n", "spacing = (3.0, 3.0, 3.0) # water spaced 3A apart in each direction\n", "\n", - "np.set_printoptions(formatter={\"float_kind\": \"{:13.10f}\".format})\n", - "\n", "waters = build_water_lattice(num_duplicates, spacing)\n", "\n", "off_crds, off_ene = evaluate_openff(waters, openff_force_field, minimize=minimize)\n", - "off_crds = np.array(off_crds.value_in_unit(openmm.unit.angstrom))\n", + "off_crds = numpy.array(off_crds.value_in_unit(openmm.unit.angstrom))\n", "\n", "omm_crds, omm_ene = evaluate_openmm(waters, openmm_force_field, minimize=minimize)\n", - "omm_crds = np.array(omm_crds.value_in_unit(openmm.unit.angstrom))\n", + "omm_crds = numpy.array(omm_crds.value_in_unit(openmm.unit.angstrom))\n", "\n", - "print_info(\n", - " np.linalg.norm(off_crds - omm_crds, axis=1),\n", - " off_ene - omm_ene,\n", - " \"OpenFF - OpenMM norm\",\n", + "coordinate_difference = Quantity(\n", + " numpy.linalg.norm(off_crds - omm_crds[reorder(numpy.prod(num_duplicates)), :]),\n", + " unit.angstrom,\n", ")\n", + "coordinate_difference /= numpy.prod(num_duplicates)\n", + "energy_difference = abs(off_ene - omm_ene) / numpy.prod(num_duplicates)\n", "\n", - "coordinate_difference = np.linalg.norm(off_crds - omm_crds) / np.prod(num_duplicates)\n", - "energy_difference = abs(off_ene - omm_ene)\n", - "\n", + "print_info(\n", + " coordinate_difference,\n", + " energy_difference,\n", + " \"OpenFF - OpenMM comparison (per molecule)\",\n", + ")\n", "\n", - "# For some reason, there is a slight coordinate/energy difference between the OpenFF\n", - "# TIP5P virtual sites and the OpenMM TIP5P virtual sites. The energy difference appears\n", - "# to be entirely due to slightly different geometry.\n", - "assert (\n", - " coordinate_difference < 0.04\n", + "assert coordinate_difference < Quantity(\n", + " 1e-6, unit.angstrom\n", "), f\"Coordinates differ by a norm of {coordinate_difference}\"\n", "assert (\n", - " energy_difference < 1.0 * openmm.unit.kilojoule_per_mole\n", + " energy_difference < 1e-4 * openmm.unit.kilojoule_per_mole\n", "), f\"Energies differ by {energy_difference}\"" ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [] } ], "metadata": { diff --git a/openff/toolkit/_tests/test_forcefield.py b/openff/toolkit/_tests/test_forcefield.py index f9baa8538..ed5a0a518 100644 --- a/openff/toolkit/_tests/test_forcefield.py +++ b/openff/toolkit/_tests/test_forcefield.py @@ -1861,6 +1861,23 @@ class BogusHandler(ParameterHandler): assert force_field["bogus"] is not None + def test_handy_handler_creation(self): + """See issue #1757""" + for key in [ + "vdW", + "Electrostatics", + "ToolkitAM1BCC", + "LibraryCharges", + "ChargeIncrementModel", + "Bonds", + "Angles", + "ProperTorsions", + "ImproperTorsions", + "VirtualSites", + "GBSA", + ]: + ForceField().get_parameter_handler(key) + class TestForceFieldPluginLoading: def test_handlers_tracked_if_already_loaded(self): diff --git a/openff/toolkit/_tests/test_parameters.py b/openff/toolkit/_tests/test_parameters.py index 7d8455e6f..9209c8680 100644 --- a/openff/toolkit/_tests/test_parameters.py +++ b/openff/toolkit/_tests/test_parameters.py @@ -2425,6 +2425,29 @@ def test_invalid_num_charge_increments(self): distance=2.0 * unit.angstrom, ) + def test_deduplicate_symmetric_matches_in_noncapturing_atoms(self): + """ + Make sure we don't double-assign vsites based on symmetries in non-tagged atoms in the SMIRKS. + See https://github.com/openforcefield/openff-toolkit/issues/1739 + """ + vsite_handler = VirtualSiteHandler(skip_version_check=True) + vsite_handler.add_parameter( + parameter_kwargs={ + "smirks": "[H][#6:2]([H])=[#8:1]", + "name": "EP", + "type": "BondCharge", + "distance": 7.0 * openff.units.unit.angstrom, + "match": "all_permutations", + "charge_increment1": 0.2 * openff.units.unit.elementary_charge, + "charge_increment2": 0.1 * openff.units.unit.elementary_charge, + "sigma": 1.0 * openff.units.unit.angstrom, + "epsilon": 2.0 * openff.units.unit.kilocalorie_per_mole, + } + ) + molecule = openff.toolkit.Molecule.from_mapped_smiles("[H:3][C:2]([H:4])=[O:1]") + matches = vsite_handler.find_matches(molecule.to_topology()) + assert len(matches) == 1 + class TestLibraryChargeHandler: def test_create_library_charge_handler(self): diff --git a/openff/toolkit/typing/engines/smirnoff/parameters.py b/openff/toolkit/typing/engines/smirnoff/parameters.py index 231409b4f..4211989cf 100644 --- a/openff/toolkit/typing/engines/smirnoff/parameters.py +++ b/openff/toolkit/typing/engines/smirnoff/parameters.py @@ -2822,7 +2822,9 @@ def to_dict( ) def __init__(self, **kwargs): - if Version(str(kwargs.get("version"))) > Version("0.3"): + if kwargs.get("version") is None: + kwargs["version"] = 0.5 + elif Version(str(kwargs.get("version"))) > Version("0.3"): if "method" in kwargs: raise SMIRNOFFSpecError( "`method` attribute has been removed in version 0.4 of the vdW tag. Use " @@ -3702,7 +3704,15 @@ def _find_matches_by_parent(self, entity: Topology) -> Dict[int, list]: matches_by_parent: Dict = defaultdict(lambda: defaultdict(list)) for parameter in self._parameters: + # Filter for redundant matches caused by non-tagged atoms + # See https://github.com/openforcefield/openff-toolkit/issues/1739 + seen_topology_atom_indices = set() for match in entity.chemical_environment_matches(parameter.smirks): + if match.topology_atom_indices in seen_topology_atom_indices: + continue + else: + seen_topology_atom_indices.add(match.topology_atom_indices) + parent_index = match.topology_atom_indices[parameter.parent_index] matches_by_parent[parent_index][parameter.name].append(