mosdef-hub / foyer

A package for atom-typing as well as applying and disseminating forcefields
https://foyer.mosdef.org
MIT License
118 stars 77 forks source link

Combining two forcefield XML files creates error #440

Closed mphoward closed 3 years ago

mphoward commented 3 years ago

Bug summary

After updating to foyer 0.9.2, I tried to apply two forcefield files within the same mbuild.save command and got this error:

/home/mphoward/software/anaconda3/envs/mosdef/lib/python3.9/site-packages/foyer/forcefield.py:879: UserWarning: Combining rule found in force field (['geometric', 'geometric']) and passed to Forcefield.apply() (lorentz) do not match. Using the rule specified in the force field
  warnings.warn(
Traceback (most recent call last):
  File "/home/mphoward/software/anaconda3/envs/mosdef/lib/python3.9/site-packages/foyer/forcefield.py", line 887, in parametrize_system
    structure.combining_rule = self.combining_rule
  File "/home/mphoward/software/anaconda3/envs/mosdef/lib/python3.9/site-packages/parmed/structure.py", line 1571, in combining_rule
    raise ValueError("combining_rule must be 'lorentz' or 'geometric'")
ValueError: combining_rule must be 'lorentz' or 'geometric'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/mphoward/Documents/projects/hydrogel_diff/scripts/mosdef/meoh/../model.py", line 543, in <module>
    make(args.outf, box, crosslinks=crosslinks, compounds=compounds)
  File "/home/mphoward/Documents/projects/hydrogel_diff/scripts/mosdef/meoh/../model.py", line 438, in make
    s.save(
  File "/home/mphoward/software/anaconda3/envs/mosdef/lib/python3.9/site-packages/mbuild/compound.py", line 2029, in save
    conversion.save(
  File "/home/mphoward/software/anaconda3/envs/mosdef/lib/python3.9/site-packages/mbuild/conversion.py", line 982, in save
    structure = ff.apply(structure, **foyer_kwargs)
  File "/home/mphoward/software/anaconda3/envs/mosdef/lib/python3.9/site-packages/foyer/forcefield.py", line 776, in apply
    return self.parametrize_system(
  File "/home/mphoward/software/anaconda3/envs/mosdef/lib/python3.9/site-packages/foyer/forcefield.py", line 891, in parametrize_system
    raise UnimplementedCombinationRuleError(
foyer.exceptions.UnimplementedCombinationRuleError: Combination rule ['geometric', 'geometric'] is not implemented

First in the stack trace, I suppose that mbuild does not currently forward the combining_rule to apply correctly:

https://github.com/mosdef-hub/mbuild/blob/c00fdc0210824c289077fe5a50eec68ceeb250cd/mbuild/conversion.py#L982-L983

since I am setting combining_rule='geometric' but the warning at forcefield.py:879 prints lorentz. Maybe that is just because it hasn't made it into a new release, though.

The main error has to do with the string equalities here:

https://github.com/mosdef-hub/foyer/blob/c8e789ba4536ce8889b9e43abb137da72483caf3/foyer/forcefield.py#L877-L896

which are incompatible with generating the combining rules as a list here:

https://github.com/mosdef-hub/foyer/blob/c8e789ba4536ce8889b9e43abb137da72483caf3/foyer/forcefield.py#L529-L542

One proposed solution would be to use a set to build up the combining rules when Forcefield is constructed, then collapse them down to a string if the set length is 1. If the has additional members, then a warning should likely be raised here because the combining rule cannot be automatically deduced.

Conceptually, it feels like combining_rule should be an option for the Forcefield constructor and should not be an argument of apply, since it is a class attribute. Specifying combining_rule in the constructor could be used to override the values from the forcefields or resolve conflicts if different forcefields give different combining rules.

Code to reproduce the behavior

Something like this would do it in mbuild:

    s.save(filename,forcefield_name='oplsaa', forcefield_files='opc.xml', combining_rule='geometric')

where opc.xml is another forcefield file I have. Or, I think the same would occur using ff.apply on the structure, but I didn't try this.

Software versions

daico007 commented 3 years ago

Thank you for alerting us of the bug @mphoward! We will be working on fixing this soon.