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

Remove convert_foyer_xml and replace with using foyer as the backend #749

Merged
merged 18 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d6bc933
deprecate any foyer to gmso conversion that does not use forcefield u…
CalCraven Aug 10, 2023
60025c0
Merge branch 'main' of https://github.com/mosdef-hub/gmso into 748-re…
CalCraven Aug 10, 2023
c6dff99
Merge branch 'main' into 748-remove-gmso-xml-conversions
CalCraven Aug 14, 2023
3ca95a2
adjust syntax in GMSO tests to load through ForceField.from_forcefiel…
CalCraven Aug 14, 2023
c36601a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 14, 2023
9813180
Add exception to catch a filenot found error and try to load via Foye…
CalCraven Aug 14, 2023
e1f4ac3
remove codecov for foyer_xml conversion in GMSO
CalCraven Aug 21, 2023
aa664e0
Merge branch 'main' into 748-remove-gmso-xml-conversions
daico007 Aug 28, 2023
9bdca83
Merge branch 'main' into 748-remove-gmso-xml-conversions
daico007 Sep 4, 2023
8e111db
Update gmso/core/forcefield.py
CalCraven Sep 5, 2023
bc900cb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 5, 2023
e19413c
Improve error handling of from xml conversions, both in GMSO and ffutils
CalCraven Sep 5, 2023
b826d48
Merge branch 'main' into 748-remove-gmso-xml-conversions
daico007 Sep 7, 2023
d93b4f3
Merge branch 'main' into 748-remove-gmso-xml-conversions
daico007 Sep 7, 2023
0b15fce
fix bug with validation error
daico007 Sep 11, 2023
87c30cb
update doc regarding python version
daico007 Sep 11, 2023
524a09b
restore filenotfound error in except clause
daico007 Sep 11, 2023
1b787dd
Merge branch 'main' into 748-remove-gmso-xml-conversions
daico007 Sep 12, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/CI.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
matrix:
os: [macOS-latest, ubuntu-latest]
python-version: ["3.8", "3.9", "3.10", "3.11"]
pydantic-version: ["1", "2"]
pydantic-version: ["2"]

defaults:
run:
Expand Down
1 change: 1 addition & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ ignore:
- "gmso/examples"
- "gmso/tests"
- "gmso/formats/networkx.py"
- "gmso/external/convert_foyer_xml.py"
4 changes: 2 additions & 2 deletions docs/installation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ Once all dependencies have been installed and the ``conda`` environment has been
Supported Python Versions
-------------------------

Python 3.7 is the recommend version for users. It is the only version on which
development and testing consistently takes place. Older (3.6) and newer (3.8+)
Python 3.8-3.11 is the recommend version for users. It is the only version on which
development and testing consistently takes place. Older (3.6-3.7) and newer (3.12+)
versions of Python 3 are likely to work but no guarantee is made and, in
addition, some dependencies may not be available for other versions. No effort
is made to support Python 2 because it is considered obsolete as of early 2020.
Expand Down
35 changes: 25 additions & 10 deletions gmso/core/forcefield.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,19 @@

from lxml import etree

try:
from pydantic.v1 import ValidationError
except:
Dismissed Show dismissed Hide dismissed
from pydantic import ValidationError

Check warning on line 14 in gmso/core/forcefield.py

View check run for this annotation

Codecov / codecov/patch

gmso/core/forcefield.py#L13-L14

Added lines #L13 - L14 were not covered by tests

from gmso.core.element import element_by_symbol
from gmso.exceptions import GMSOError, MissingPotentialError
from gmso.exceptions import (
ForceFieldParseError,
GMSOError,
MissingPotentialError,
)
from gmso.utils._constants import FF_TOKENS_SEPARATOR
from gmso.utils.decorators import deprecate_kwargs
from gmso.utils.decorators import deprecate_function, deprecate_kwargs
from gmso.utils.ff_utils import (
parse_ff_atomtypes,
parse_ff_connection_types,
Expand Down Expand Up @@ -45,10 +54,8 @@

Parameters
----------
name : str
Name of the forcefield, default 'ForceField'
version : str
a cannonical semantic version of the forcefield, default 1.0.0
xml_loc : str
Path to the forcefield xml. The forcefield xml can be either in Foyer or GMSO style.
strict: bool, default=True
If true, perform a strict validation of the forcefield XML file
greedy: bool, default=True
Expand Down Expand Up @@ -104,6 +111,7 @@
"ffutils",
]:
ff = ForceField.xml_from_forcefield_utilities(xml_loc)

else:
raise (
GMSOError(
Expand Down Expand Up @@ -565,10 +573,14 @@

@classmethod
def xml_from_forcefield_utilities(cls, filename):
from forcefield_utilities.xml_loader import GMSOFFs

loader = GMSOFFs()
ff = loader.load(filename).to_gmso_ff()
from forcefield_utilities.xml_loader import FoyerFFs, GMSOFFs

try:
loader = GMSOFFs()
ff = loader.load(filename).to_gmso_ff()
except (ForceFieldParseError, FileNotFoundError, ValidationError):
loader = FoyerFFs()
ff = loader.load(filename).to_gmso_ff()
return ff

def to_xml(self, filename, overwrite=False, backend="gmso"):
Expand Down Expand Up @@ -723,6 +735,9 @@
)

@classmethod
@deprecate_function(
"The internal `from_xml` will be deprecated soon. Please load the XML with the `xml_from_forcefield_utilities`."
)
def from_xml(cls, xmls_or_etrees, strict=True, greedy=True):
"""Create a gmso.Forcefield object from XML File(s).

Expand Down
1 change: 0 additions & 1 deletion gmso/external/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Support for various in-memory representations of chemical systems."""
from .convert_foyer_xml import from_foyer_xml
from .convert_hoomd import (
to_gsd_snapshot,
to_hoomd_forcefield,
Expand Down
4 changes: 4 additions & 0 deletions gmso/external/convert_foyer_xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@
from lxml import etree

from gmso.exceptions import ForceFieldParseError
from gmso.utils.decorators import deprecate_function


@deprecate_function(
"The `from_foyer_xml` method will be deprecated soon. Please use the package `forcefield-utilities.FoyerFFs`."
)
def from_foyer_xml(
foyer_xml, gmso_xml=None, overwrite=False, validate_foyer=False
):
Expand Down
34 changes: 6 additions & 28 deletions gmso/tests/base_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import forcefield_utilities as ffutils
import foyer
import mbuild as mb
import numpy as np
Expand All @@ -17,7 +16,6 @@
from gmso.core.pairpotential_type import PairPotentialType
from gmso.core.topology import Topology
from gmso.external import from_mbuild, from_parmed
from gmso.external.convert_foyer_xml import from_foyer_xml
from gmso.parameterization import apply
from gmso.tests.utils import get_path
from gmso.utils.io import get_fn
Expand Down Expand Up @@ -74,11 +72,7 @@ def benzene_ua_box(self):
@pytest.fixture
def typed_benzene_ua_system(self, benzene_ua_box):
top = benzene_ua_box
trappe_benzene = (
ffutils.FoyerFFs()
.load(get_path("benzene_trappe-ua.xml"))
.to_gmso_ff()
)
trappe_benzene = ForceField(get_path("benzene_trappe-ua.xml"))
top = apply(top=top, forcefields=trappe_benzene, remove_untyped=True)
return top

Expand All @@ -104,7 +98,7 @@ def benzene_aa_box(self):
@pytest.fixture
def typed_benzene_aa_system(self, benzene_aa_box):
top = benzene_aa_box
oplsaa = ffutils.FoyerFFs().load("oplsaa").to_gmso_ff()
oplsaa = ForceField("oplsaa")
top = apply(top=top, forcefields=oplsaa, remove_untyped=True)
return top

Expand Down Expand Up @@ -273,41 +267,25 @@ def typed_water_system(self, water_system):
def foyer_fullerene(self):
from foyer.tests.utils import get_fn

from_foyer_xml(get_fn("fullerene.xml"), overwrite=True)
gmso_ff = ForceField("fullerene_gmso.xml")

return gmso_ff
return ForceField(get_fn("fullerene.xml"))

@pytest.fixture
def foyer_periodic(self):
# TODO: this errors out with backend="ffutils"
from foyer.tests.utils import get_fn

from_foyer_xml(get_fn("oplsaa-periodic.xml"), overwrite=True)
gmso_ff = ForceField("oplsaa-periodic_gmso.xml", backend="gmso")

return gmso_ff
return ForceField(get_fn("oplsaa-periodic.xml"))

@pytest.fixture
def foyer_urey_bradley(self):
# TODO: this errors out with backend="ffutils"
from foyer.tests.utils import get_fn

from_foyer_xml(get_fn("charmm36_cooh.xml"), overwrite=True)
gmso_ff = ForceField("charmm36_cooh_gmso.xml", backend="gmso")

return gmso_ff
return ForceField(get_fn("charmm36_cooh.xml"))

@pytest.fixture
def foyer_rb_torsion(self):
from foyer.tests.utils import get_fn

from_foyer_xml(
get_fn("refs-multi.xml"), overwrite=True, validate_foyer=True
)
gmso_ff = ForceField("refs-multi_gmso.xml")

return gmso_ff
return ForceField(get_fn("refs-multi.xml"))

@pytest.fixture
def methane(self):
Expand Down
37 changes: 13 additions & 24 deletions gmso/tests/test_convert_foyer_xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

from gmso.core.forcefield import ForceField
from gmso.exceptions import ForceFieldParseError
from gmso.external.convert_foyer_xml import from_foyer_xml
from gmso.tests.base_test import BaseTest
from gmso.tests.utils import get_path

Expand All @@ -18,51 +17,40 @@ class TestXMLConversion(BaseTest):
def test_from_foyer(self, ff):
from foyer.tests.utils import get_fn

from_foyer_xml(get_fn(ff), overwrite=True)

@pytest.mark.parametrize("ff", parameterized_ffs)
def test_from_foyer_overwrite_false(self, ff):
from foyer.tests.utils import get_fn

from_foyer_xml(get_fn(ff), overwrite=False)
with pytest.raises(FileExistsError):
from_foyer_xml(get_fn(ff), overwrite=False)
ForceField(get_fn(ff))

@pytest.mark.parametrize("ff", parameterized_ffs)
def test_from_foyer_different_name(self, ff):
from foyer.tests.utils import get_fn

from_foyer_xml(get_fn(ff), f"{ff}-gmso-converted.xml", overwrite=True)
ForceField(get_fn(ff), f"{ff}-gmso-converted.xml")

@pytest.mark.parametrize("ff", parameterized_ffs)
def test_from_foyer_validate_foyer(self, ff):
from foyer.tests.utils import get_fn

from_foyer_xml(
ForceField(
get_fn(ff),
f"{ff}-gmso-converted.xml",
overwrite=True,
validate_foyer=True,
)

@pytest.mark.parametrize("ff", parameterized_ffs)
def test_foyer_pathlib(self, ff):
from foyer.tests.utils import get_fn

file_path = Path(get_fn(ff))
from_foyer_xml(file_path, overwrite=True)
ForceField(file_path)

def test_foyer_file_not_found(self):
file_path = "dummy_name.xml"
with pytest.raises(FileNotFoundError):
from_foyer_xml(file_path, overwrite=True)
ForceField(file_path)

def test_foyer_version(self, foyer_fullerene):
assert foyer_fullerene.version == "0.0.1"
assert foyer_fullerene.version == "1.0.0"

def test_foyer_combining_rule(self):
from_foyer_xml(get_path("foyer-trappe-ua.xml"))
loaded = ForceField("foyer-trappe-ua_gmso.xml")
loaded = ForceField(get_path("foyer-trappe-ua.xml"))

assert loaded.name == "Trappe-UA"
assert loaded.version == "0.0.2"
Expand Down Expand Up @@ -109,7 +97,7 @@ def test_foyer_bonds(self, foyer_fullerene):
assert "C~C" in foyer_fullerene.bond_types

assert foyer_fullerene.bond_types["C~C"].expression == sympify(
"1/2 * k * (r-r_eq)**2"
"0.5 * k * (r-r_eq)**2"
)
assert (
sympify("r")
Expand All @@ -128,7 +116,7 @@ def test_foyer_angles(self, foyer_fullerene):
assert "C~C~C" in foyer_fullerene.angle_types

assert foyer_fullerene.angle_types["C~C~C"].expression == sympify(
"1/2 * k * (theta - theta_eq)**2"
"0.5 * k * (theta - theta_eq)**2"
)
assert (
sympify("theta")
Expand Down Expand Up @@ -167,7 +155,7 @@ def test_foyer_dihedrals(self, foyer_periodic):
].parameters["n"] == u.unyt_quantity(1, u.dimensionless)
assert foyer_periodic.dihedral_types[
"opls_140~opls_135~opls_135~opls_140"
].parameters["delta"] == u.unyt_quantity(3.14, u.rad)
].parameters["phi_eq"] == u.unyt_quantity(3.14, u.rad)
assert foyer_periodic.dihedral_types[
"opls_140~opls_135~opls_135~opls_140"
].member_types == ("opls_140", "opls_135", "opls_135", "opls_140")
Expand All @@ -179,5 +167,6 @@ def test_foyer_rb_torsion(self, foyer_rb_torsion):
assert foyer_rb_torsion.dihedral_types["HC~CT~CT~HC"] is not None

def test_empty_foyer_atomtype(self):
with pytest.raises(ForceFieldParseError):
from_foyer_xml(get_path("empty_foyer.xml"))
with pytest.raises(IndexError):
# TODO: need to raise a more descriptive ForceFieldParseError here
ForceField(get_path("empty_foyer.xml"))
8 changes: 5 additions & 3 deletions gmso/tests/test_forcefield.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@
assert ff.pairpotential_types["Xe~Xe"].member_types == ("Xe", "Xe")

def test_ff_charmm_xml(self):
charm_ff = ForceField(get_path("trimmed_charmm.xml"), backend="gmso")
charm_ff = ForceField(get_path("trimmed_charmm.xml"), backend="ffutils")

assert charm_ff.name == "topologyCharmm"
assert "*~CS~SS~*" in charm_ff.dihedral_types
Expand All @@ -252,8 +252,10 @@
)

def test_non_unique_params(self):
with pytest.raises(DocumentInvalid):
ForceField(get_path("ff-example-nonunique-params.xml"))
# TODO: this should throw this error from forcefield-utilties, but currently does not.
# with pytest.raises(DocumentInvalid):
# ForceField(get_path("ff-example-nonunique-params.xml"))
Comment on lines +256 to +257

Check notice

Code scanning / CodeQL

Commented-out code Note test

This comment appears to contain commented-out code.
pass

def test_missing_params(self):
# TODO: raise same error if backend loader is forcefield-utilities
Expand Down
13 changes: 8 additions & 5 deletions gmso/tests/test_xml_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import os

import pytest
from forcefield_utilities import GMSOFFs

from gmso.core.forcefield import ForceField
from gmso.tests.base_test import BaseTest
Expand Down Expand Up @@ -67,7 +66,7 @@ def test_write_xml_from_topology(self):
@pytest.mark.parametrize("xml", TEST_XMLS)
def test_load__direct_from_forcefield_utilities(self, xml):
"""Validate loaded xmls from ff-utils match original file."""
ff = GMSOFFs().load_xml(xml).to_gmso_ff()
ff = ForceField(xml)
assert isinstance(ff, ForceField)

@pytest.mark.parametrize("xml", TEST_XMLS)
Expand Down Expand Up @@ -98,16 +97,20 @@ def test_load_write_xmls_ffutils_backend(self, xml):
"""Validate loaded xmls written out match original file."""
ff1 = ForceField(xml, backend="forcefield-utilities")
ff1.to_xml("tmp.xml", overwrite=True)
ff2 = GMSOFFs().load_xml("tmp.xml").to_gmso_ff()
ff2 = ForceField("tmp.xml")
if "test_ffstyles" not in xml:
assert compare_xml_files("tmp.xml", xml)
assert ff1 == ff2

def test_xml_error_handling(self):
"""Validate bad xml formatting in xmls."""
pass
file_path = "dummy_name.xml"
with pytest.raises(FileNotFoundError):
ForceField(file_path)
with pytest.raises(IndexError):
ForceField(get_path("empty_foyer.xml"))

def test_kb_in_ffutils(self):
xml_path = get_path("ff-example0.xml")
ff = ForceField(xml_path, backend="forcefield-utilities")
ff = ForceField(xml_path)
assert ff
14 changes: 14 additions & 0 deletions gmso/utils/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,17 @@ def _inner(*args, **kwargs):
return _inner

return _function_wrapper


def deprecate_function(msg, klass=PendingDeprecationWarning):
"""Raise a warning that a given function will be deprecated soon."""

def decorator(func):
@functools.wraps(func)
def wrapper(*args, **kwargs):
warnings.warn(msg, klass, stacklevel=2)
return func(*args, **kwargs)

return wrapper

return decorator
Loading
Loading