Add utilities for parsing gmso forcefields XML. Closes #4

[umesh-timalsina]

No description provided.

[codecov]

Codecov Report

Merging #5 (135bf73) into main (096c1e8) will increase coverage by 4.23%.
The diff coverage is 95.61%.

@@            Coverage Diff             @@
##             main       #5      +/-   ##
==========================================
+ Coverage   86.60%   90.84%   +4.23%     
==========================================
  Files           9       12       +3     
  Lines         702     1322     +620     
==========================================
+ Hits          608     1201     +593     
- Misses         94      121      +27     

Impacted Files	Coverage Δ
forcefield_utilities/tests/utils.py	83.33% <83.33%> (ø)
forcefield_utilities/utils.py	92.68% <92.85%> (+0.09%)	⬆️
forcefield_utilities/gmso_xml.py	94.28% <94.28%> (ø)
forcefield_utilities/xml_loader.py	95.06% <94.87%> (+0.14%)	⬆️
forcefield_utilities/__init__.py	100.00% <100.00%> (ø)
forcefield_utilities/tests/test_gmso_xml.py	100.00% <100.00%> (ø)
forcefield_utilities/tests/test_xml_loader.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[daico007]

I think this looks good! Have a few questions, but nothing major. I think this is ready to be merged (or after you add the remaining test @umesh-timalsina)

[umesh-timalsina]

Depends on mosdef-hub/gmso#681

[CalCraven]

Going to push a few tests here and in GMSO for handling the reading for these functions. We should wait on this until those are all implemented. I think we're going to transition in the next week to a full dependency on forcefield-utilities for GMSO.

[CalCraven]

This doesn't work if the type or class is only partially defined e.g.

<DihedralType name="DihedralType1" type1="" type2="C" type3="C" type4="">

We need to switch the and's for or's here to check if any one is defined.

[umesh-timalsina]

@CalCraven Good catch. I think we need an alternative approach where in the default value is populated as a * and this check should stay as is.

[daico007]

mosdef-hub/gmso#681 is merged.

[CalCraven]

I'm having an issue with the defaults unyts K*kb for energy when loading these forcefields. The GMSO inhouse reader can do it fine, so I think it's something we need to fix here before merging.

[CalCraven]

 def test_ffutils_backend(self):
        ff = ForceField()
>       ff.load_backend_forcefield_utilities(get_path("ff-example0.xml"))

self = FFMetaData(children=[Units(energy='K*kb', distance='nm', mass='amu', charge='coulomb', temperature=None, angle=None, time=None)], electrostatics14Scale=0.5, nonBonded14Scale=0.67, combining_rule='lorentz')

    def get_default_units(self):
        units_dict = {}
        units = self.children[0].dict(by_alias=True, exclude_none=True)
        for name, unit in units.items():
            try:
                units_dict[name] = u.Unit(unit)
            except u.exceptions.UnitParseError:
>               units_dict[name] = getattr(u.physical_constants, unit)
E               AttributeError: module 'unyt.physical_constants' has no attribute 'K*kb'

def load_backend_forcefield_utilities(self, filename, backend="forcefield-utilities"):
     from forcefield_utilities.xml_loader import GMSOFFs
     loader = GMSOFFs()
     self = loader.load(filename).to_gmso_ff()

[CalCraven]

For now, I will test forcefields in GMSO that don't have the K*kb unit styles. Once this PR is merged, we should fix that functionality.

[umesh-timalsina]

@CalCraven I think this is fine to do as long as we define a new unit called _kb and similar units that would be good. Then we would either extend the existing unyt registry or define our own for find and replace. Since kb is already defined and is a physical_quantity in unyt. Similar utilities were developed in the inhouse reader which can be added here but we need a listing of relevant physical quantities that are used interchangeably as units.

>>> import unyt as U
>>>  U.define_unit('_kb', 1.0 * U.kb)

[umesh-timalsina]

This is still incomplete with duplicates trimming. Because of multiple sections allowed. An alternate strategy is necessary to weedout duplicates.

[CalCraven]

To be clear, duplicates means duplicate section headers in the XML. I would expect duplicated sections to just append to that object in the GMSO ForceField class, but if it's going to take a bit more time to implement, I would suggest merging this as is and document that we only support a single section for now, pending a new PR for it.

[daico007]

Agree with @CalCraven that we should merge this PR in the interest of time, and we can list the remaining to do as issue to be deal with in near future.

[umesh-timalsina]

I am working on an async coroutine to make sure we error out on any duplicates encountered. Should be complete soon.