Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1216 #1290

Merged
merged 6 commits into from
May 6, 2022
Merged

Fix #1216 #1290

merged 6 commits into from
May 6, 2022

Conversation

j-wags
Copy link
Member

@j-wags j-wags commented Apr 29, 2022

@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #1290 (99c7d29) into master (27ef1d7) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

❗ Current head 99c7d29 differs from pull request most recent head b35f883. Consider uploading reports for the commit b35f883 to get more accurate results

Copy link
Member

@mattwthompson mattwthompson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This passes the repros in #1216 and #1047, but we should add a test. The issues are roughly duplicates so I one test will suffice.

The parameters in #1047 aren't supported verbatim in after the rework so #1216 is probably quicker to throw in. (I didn't look into this further, it seems tangential to the focus of the fix.)

(openff-dev) [openff-toolkit] git checkout 0.10.6                                           7:18:28  ☁  2af5b53a ☂
Previous HEAD position was 2af5b53a Use `packaging.version` to manage SMIRNOFF section versions (#1279)
HEAD is now at 37d6806b Fix #1283 (#1284)
(openff-dev) [openff-toolkit] python 1047.py                                                7:18:32  ☁  37d6806b ☂
Warning: Unable to load toolkit 'OpenEye Toolkit'. The Open Force Field Toolkit does not require the OpenEye Toolkits, and can use RDKit/AmberTools instead. However, if you have a valid license for the OpenEye Toolkits, consider installing them for faster performance and additional file format support: https://docs.eyesopen.com/toolkits/python/quickstart-python/linuxosx.html OpenEye offers free Toolkit licenses for academics: https://www.eyesopen.com/academic-licensing
<openff.toolkit.topology.topology.ValenceDict object at 0x162a67370>
<openff.toolkit.topology.topology.ValenceDict object at 0x162c5db80>
<openff.toolkit.topology.topology.TagSortedDict object at 0x162c5dbe0>
[<openff.toolkit.typing.engines.smirnoff.parameters.ParameterHandler._Match object at 0x162c5dfa0>]
Traceback (most recent call last):
  File "/Users/mattthompson/software/openff-toolkit/1047.py", line 58, in <module>
    print(force_field.label_molecules(Molecule.from_smiles("CF").to_topology()))
  File "/Users/mattthompson/software/openff-toolkit/openff/toolkit/typing/engines/smirnoff/forcefield.py", line 1513, in label_molecules
    m.parameter_type for m in matches[match]
TypeError: list indices must be integers or slices, not _Match
(openff-dev) [openff-toolkit] python 1216.py                                                7:18:35  ☁  37d6806b ☂
Warning: Unable to load toolkit 'OpenEye Toolkit'. The Open Force Field Toolkit does not require the OpenEye Toolkits, and can use RDKit/AmberTools instead. However, if you have a valid license for the OpenEye Toolkits, consider installing them for faster performance and additional file format support: https://docs.eyesopen.com/toolkits/python/quickstart-python/linuxosx.html OpenEye offers free Toolkit licenses for academics: https://www.eyesopen.com/academic-licensing
<openff.toolkit.topology.topology.ValenceDict object at 0x164799580>
[<openff.toolkit.typing.engines.smirnoff.parameters.ParameterHandler._Match object at 0x16478d0a0>]
Traceback (most recent call last):
  File "/Users/mattthompson/software/openff-toolkit/1216.py", line 45, in <module>
    print(force_field.label_molecules(Molecule.from_smiles("CF").to_topology()))
  File "/Users/mattthompson/software/openff-toolkit/openff/toolkit/typing/engines/smirnoff/forcefield.py", line 1513, in label_molecules
    m.parameter_type for m in matches[match]
TypeError: list indices must be integers or slices, not _Match
(openff-dev) [openff-toolkit] git checkout fix-1216                                         7:18:39  ☁  37d6806b ☂
Previous HEAD position was 37d6806b Fix #1283 (#1284)
Switched to branch 'fix-1216'
(openff-dev) [openff-toolkit] python 1047.py                                                7:18:48  ☁  fix-1216 ☂
Warning: Unable to load toolkit 'OpenEye Toolkit'. The Open Force Field Toolkit does not require the OpenEye Toolkits, and can use RDKit/AmberTools instead. However, if you have a valid license for the OpenEye Toolkits, consider installing them for faster performance and additional file format support: https://docs.eyesopen.com/toolkits/python/quickstart-python/linuxosx.html OpenEye offers free Toolkit licenses for academics: https://www.eyesopen.com/academic-licensing
[{'Constraints': <openff.toolkit.topology.topology.ValenceDict object at 0x1629ec910>, 'Electrostatics': <openff.toolkit.topology.topology.ValenceDict object at 0x162d0dac0>, 'ChargeIncrementModel': <openff.toolkit.topology.topology.TagSortedDict object at 0x1629ec1c0>, 'VirtualSites': {(0,): [<VirtualSiteType with smirks: [#6:1][#9:2]  epsilon: 0.0 kcal/mol  type: BondCharge  match: all_permutations  distance: 1.0 A  outOfPlaneAngle: None  inPlaneAngle: None  charge_increment1: 0.2 e  charge_increment2: 0.1 e  sigma: 1.0 A  name: EP  >]}, 'vdW': <openff.toolkit.topology.topology.ValenceDict object at 0x1629ec6a0>}]
(openff-dev) [openff-toolkit] python 1216.py                                                7:18:51  ☁  fix-1216 ☂
Warning: Unable to load toolkit 'OpenEye Toolkit'. The Open Force Field Toolkit does not require the OpenEye Toolkits, and can use RDKit/AmberTools instead. However, if you have a valid license for the OpenEye Toolkits, consider installing them for faster performance and additional file format support: https://docs.eyesopen.com/toolkits/python/quickstart-python/linuxosx.html OpenEye offers free Toolkit licenses for academics: https://www.eyesopen.com/academic-licensing
[{'Electrostatics': <openff.toolkit.topology.topology.ValenceDict object at 0x164f3d130>, 'VirtualSites': {(0,): [<VirtualSiteType with smirks: [#6:1][#9:2]  epsilon: 0.0 kcal/mol  type: BondCharge  match: all_permutations  distance: 1.0 A  outOfPlaneAngle: None  inPlaneAngle: None  charge_increment1: 0.2 e  charge_increment2: 0.1 e  sigma: 1.0 A  name: EP  >]}, 'LibraryCharges': {(0, 1, 2, 3, 4): <LibraryChargeType with smirks: [F:2][C:1]([H:3])([H:4])([H:5])  charge1: 0.3 e  charge2: -0.15 e  charge3: -0.05 e  charge4: -0.05 e  charge5: -0.05 e  >, (0, 1, 2, 4, 3): <LibraryChargeType with smirks: [F:2][C:1]([H:3])([H:4])([H:5])  charge1: 0.3 e  charge2: -0.15 e  charge3: -0.05 e  charge4: -0.05 e  charge5: -0.05 e  >, (0, 1, 3, 2, 4): <LibraryChargeType with smirks: [F:2][C:1]([H:3])([H:4])([H:5])  charge1: 0.3 e  charge2: -0.15 e  charge3: -0.05 e  charge4: -0.05 e  charge5: -0.05 e  >, (0, 1, 3, 4, 2): <LibraryChargeType with smirks: [F:2][C:1]([H:3])([H:4])([H:5])  charge1: 0.3 e  charge2: -0.15 e  charge3: -0.05 e  charge4: -0.05 e  charge5: -0.05 e  >, (0, 1, 4, 2, 3): <LibraryChargeType with smirks: [F:2][C:1]([H:3])([H:4])([H:5])  charge1: 0.3 e  charge2: -0.15 e  charge3: -0.05 e  charge4: -0.05 e  charge5: -0.05 e  >, (0, 1, 4, 3, 2): <LibraryChargeType with smirks: [F:2][C:1]([H:3])([H:4])([H:5])  charge1: 0.3 e  charge2: -0.15 e  charge3: -0.05 e  charge4: -0.05 e  charge5: -0.05 e  >}}]
(openff-dev) [openff-toolkit] cat 1*y                                                       7:18:55  ☁  fix-1216 ☂
from openff.toolkit.topology import Molecule
from openff.toolkit.typing.engines.smirnoff import ForceField
from openmm import unit as simtk_unit

# first build tip4p water with vsites before vdw
force_field = ForceField()
constraint_handler = force_field.get_parameter_handler("Constraints")
# Keep the H-O bond length fixed at 0.9572 angstroms.
constraint_handler.add_parameter(
    {"smirks": "[#1:1]-[#8X2H2+0:2]-[#1]", "distance": 0.9572 * simtk_unit.angstrom}
)
# Keep the H-O-H angle fixed at 104.52 degrees.
constraint_handler.add_parameter(
    {"smirks": "[#1:1]-[#8X2H2+0]-[#1:2]", "distance": 1.5139 * simtk_unit.angstrom}
)

force_field.get_parameter_handler("Electrostatics")

force_field.get_parameter_handler(
    "ChargeIncrementModel",
    {"version": "0.3", "partial_charge_method": "formal_charge"},
)
from openff.toolkit.typing.engines.smirnoff import ParameterList

virtual_site_handler = force_field.get_parameter_handler("VirtualSites")
virtual_site_handler.add_parameter(
    {
        "smirks": "[#6:1][#9:2]",
        "name": "EP",
        "type": "BondCharge",
        "distance": 1.0 * simtk_unit.angstrom,
        "match": "all_permutations",
        "charge_increment1": 0.2 * simtk_unit.elementary_charge,
        "charge_increment2": 0.1 * simtk_unit.elementary_charge,
        "sigma": 1.0 * simtk_unit.angstrom,
        "epsilon": 0.0 * simtk_unit.kilocalorie_per_mole,
    }
)
# Currently required due to OpenFF issue #884
virtual_site_handler._parameters = ParameterList(virtual_site_handler._parameters)

vdw_handler = force_field.get_parameter_handler("vdW")
vdw_handler.add_parameter(
    {
        "smirks": "[#6:1][#9:2]",
        "epsilon": 0.0 * simtk_unit.kilojoule_per_mole,
        "sigma": 1.0 * simtk_unit.angstrom,
    }
)
vdw_handler.add_parameter(
    {
        "smirks": "[#6:2][#9:1]",
        "epsilon": 0.155 * simtk_unit.kilocalorie_per_mole,
        "sigma": 3.15365 * simtk_unit.angstroms,
    }
)

print(force_field.label_molecules(Molecule.from_smiles("CF").to_topology()))
from openff.toolkit.typing.engines.smirnoff import ForceField
from openff.toolkit.topology import Molecule
from openff.toolkit.typing.engines.smirnoff.parameters import (
    LibraryChargeHandler,
    VirtualSiteHandler
)
from simtk import unit as simtk_unit

force_field = ForceField()
force_field.get_parameter_handler("Electrostatics")

vsite_handler: VirtualSiteHandler = force_field.get_parameter_handler(
    "VirtualSites"
)
vsite_handler.add_parameter(
    {
        "smirks": "[#6:1][#9:2]",
        "name": "EP",
        "type": "BondCharge",
        "distance": 1.0 * simtk_unit.angstrom,
        "match": "all_permutations",
        "charge_increment1": 0.2 * simtk_unit.elementary_charge,
        "charge_increment2": 0.1 * simtk_unit.elementary_charge,
        "sigma": 1.0 * simtk_unit.angstrom,
        "epsilon": 0.0 * simtk_unit.kilocalorie_per_mole,
    }
)

library_handler: LibraryChargeHandler = force_field.get_parameter_handler(
    "LibraryCharges"
)
library_handler.add_parameter(
    {
        "smirks": "[F:2][C:1]([H:3])([H:4])([H:5])",
        "charge": [
            0.3 * simtk_unit.elementary_charge,
            -0.15 * simtk_unit.elementary_charge,
            -0.05 * simtk_unit.elementary_charge,
            -0.05 * simtk_unit.elementary_charge,
            -0.05 * simtk_unit.elementary_charge
        ]
    }
)

print(force_field.label_molecules(Molecule.from_smiles("CF").to_topology()))

@j-wags
Copy link
Member Author

j-wags commented May 6, 2022

Thanks @mattwthompson - I will add a test for this and then merge.

@j-wags j-wags merged commit fade767 into master May 6, 2022
@j-wags j-wags deleted the fix-1216 branch May 6, 2022 22:19
mattwthompson added a commit that referenced this pull request May 9, 2022
* change vsitehandler.find_matches to be more in theme with base find_matches return values

* spurious print cleanup and releasenotes

* fixes for mypy

* Fix a type annotation

* add test

* black

Co-authored-by: Matthew W. Thompson <mattwthompson@protonmail.com>
mattwthompson added a commit that referenced this pull request May 9, 2022
* Fix #1216 (#1290)

* change vsitehandler.find_matches to be more in theme with base find_matches return values

* spurious print cleanup and releasenotes

* fixes for mypy

* Fix a type annotation

* add test

* black

Co-authored-by: Matthew W. Thompson <mattwthompson@protonmail.com>

* Update test given `VirtualSiteHandler.find_matches -> List`

Co-authored-by: Jeff Wagner <jwagnerjpl@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants