Add get_parameters function to Forcefield class

[umesh-timalsina]

This PR is related to #381. Add specific parameter extraction functions to a foyer ForceField.

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 4fcc69ef7a5004c6fd2024fbefeeeac2eea35ad2 into 82ebfca79044af9f2fb449b91941fdc8071e6c5d - view on LGTM.com

new alerts:

• 1 for Unused local variable
• 1 for Unused import

[umesh-timalsina]

With the antefoyer installed (thanks @rsdefever), This PR is now ready for review and some testing.

[umesh-timalsina]

Currently, I have written parameters extraction for the following:

1. atoms: NonBondedForce
2. bonds: HarmonicBondForce
3. angles: HarmonicAngleForce
4. periodic_propers, periodic_impropers: PeriodicTorsionForce
5. rb_propers, rb_impropers: RBTorsionForce

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 4a223816f531660bb8b70f9ddbf4377cc0343b84 into 82ebfca79044af9f2fb449b91941fdc8071e6c5d - view on LGTM.com

new alerts:

• 1 for Syntax error

[umesh-timalsina]

@CalCraven. Thank You for your review.

I can't quite follow what ff.getGenerators() is doing. But might want to take a look at that usage.

Yeah. It returns a list of Generators for the Forcefield.

Get the following error: oplsaa.get_parameters("periodic_impropers", ["opls_149","opls_149","opls_149","opls_149"]) Traceback (most recent call last): File "", line 1, in File "/Users/calcraven/Documents/Vanderbilt/Research/mosdef_PR_Reviews/foyer/foyer/forcefield.py", line 1010, in get_parameters return param_extractorsgroup File "/Users/calcraven/Documents/Vanderbilt/Research/mosdef_PR_Reviews/foyer/foyer/forcefield.py", line 1156, in _extract_periodic_improper_params ff=self, gen_type=PeriodicTorsionGenerator File "/Users/calcraven/Documents/Vanderbilt/Research/mosdef_PR_Reviews/foyer/foyer/forcefield.py", line 1301, in get_generator return list(filter(lambda x: isinstance(x, gen_type), ff.getGenerators())).pop() IndexError: pop from empty list

I have fixed this error.

Another nice use case might be to fix the error that pops up when passing a list with a single element to the key parameter in get_parameters when searching for nonbonded atom group parameters. Current use gives the expected output: atoolsff.get_parameters("atoms", "opls_1006") {'charge': 0.418, 'epsilon': 0.0, 'sigma': 1.0}

The support has been added.

Whereas it might be useful to allow for this use to return the same output, since other inputs for groups ('angles','bonds', etc) are input as a list of strings and this would allow for consistency.

I'm also getting some strange output when trying to use atom_classes to define my bonds. oplsaa = foyer.Forcefield(name="oplsaa") oplsaa.get_parameters("bonds", ["OW","HW"],keys_are_atom_classes = True) Traceback (most recent call last): File "", line 1, in File "/Users/calcraven/Documents/Vanderbilt/Research/mosdef_PR_Reviews/foyer/foyer/forcefield.py", line 1010, in get_parameters return param_extractorsgroup File "/Users/calcraven/Documents/Vanderbilt/Research/mosdef_PR_Reviews/foyer/foyer/forcefield.py", line 1059, in _extract_harmonic_bond_params f"Missing HarmonicBond parameters for bond between " foyer.exceptions.MissingParametersError: Missing HarmonicBond parameters for bond between atoms OW and HW

This will not work because the atomClass OW is missing from the AtomTypes section in the ForceField.

[umesh-timalsina]

@rsdefever I have added the appropriate tests for testing reverse/forward keys.

Edit: Another feature that might be nice is the ability to print the line from the XML file where the parameters were taken from. E.g.,

<Improper class1="c2" class2="c" class3="c2" class4="c3" periodicity1="2" k1="4.6024" phase1="3.141592653589793"/>

This would make debugging much easier, and make it so you could quickly see if the parameters came from a wildcard match or if they were fully specified.

While it is certainly a nice feature to have. Diving into the OpenMM codebase, its not super straight forward to get the line in the XML file. While, it could be done by implementing a custom XML search, I think this feature should be added in a different PR(as this will not make use of any OpenMM forcefield specefic feature AFAIK. So, I think we could defer having this to the next issue/PR. Thoughts?

[codecov[bot]]

Codecov Report

Merging #382 (3e8bd15) into master (7816bf5) will increase coverage by 0.50%. The diff coverage is 89.01%.

:exclamation: Current head 3e8bd15 differs from pull request most recent head 7176cda. Consider uploading reports for the commit 7176cda to get more accurate results

@@            Coverage Diff             @@
##           master     #382      +/-   ##
==========================================
+ Coverage   84.57%   85.08%   +0.50%     
==========================================
  Files          14       15       +1     
  Lines        1277     1448     +171     
==========================================
+ Hits         1080     1232     +152     
- Misses        197      216      +19     

[umesh-timalsina]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).