openforcefield / smirnoff99Frosst

A general small molecule force field descended from AMBER99 and parm@Frosst, available in the SMIRNOFF format
Creative Commons Attribution 4.0 International
28 stars 9 forks source link

Remove generic parameters from this ffxml file #72

Closed bannanc closed 6 years ago

bannanc commented 6 years ago

Per discussion on openforcefield issue#75 we want to remove generic parameters from our ffxml files.

slochower commented 6 years ago

I ran across a wildcard error tonight: SMIRKSParsingError: (SMIRKSParsingError(...), 'Error SMIRKS ([*:1]~[*:2]~[*:3]~[*:4]) was not parseable into a ChemicalEnvironment'). I have not reduced the problem to something simple, so I don't have an easy, concise example -- but is it possible that removing the generic parameters left some vestiges somewhere else in the code?

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/chemistry/environment.py in __init__(self, smirks, label, replacements)
    466             try:
--> 467                 self._parse_smirks(smirks)
    468             except:

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/chemistry/environment.py in _parse_smirks(self, input_smirks)
    592                     bondANDtypes=bAND, newORtypes=aOR, newANDtypes=aAND,
--> 593                     newAtomIndex=index, newAtomRing=ring, beyondBeta=True)
    594 

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/chemistry/environment.py in addAtom(self, bondToAtom, bondORtypes, bondANDtypes, newORtypes, newANDtypes, newAtomIndex, newAtomRing, beyondBeta)
    917         # create new bond
--> 918         newBond = self.Bond(bondORtypes, bondANDtypes)
    919 

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/chemistry/environment.py in __init__(self, ORtypes, ANDtypes)
    346             """
--> 347             super(ChemicalEnvironment.Bond,self).__init__(ORtypes, ANDtypes, None, None)
    348             self._atom = False

TypeError: super(type, obj): obj must be an instance or subtype of type

During handling of the above exception, another exception occurred:

SMIRKSParsingError                        Traceback (most recent call last)
<ipython-input-33-4af53ed20f7e> in <module>()
     24     convert(destination, name, 
     25             reference_prmtop, reference_inpcrd,
---> 26             host_resname, guest_resname)

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/smirnovert/convert.py in convert(destination, prefix, reference_prmtop, reference_inpcrd, host_resname, guest_resname, debug)
    117     molecules = [host, guest]
    118 
--> 119     ff = ForceField('forcefield/smirnoff99Frosst.ffxml')
    120     system = ff.createSystem(
    121         hg_topology.topology,

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/engines/smirnoff/forcefield.py in __init__(self, *files)
    532         """
    533         self._forces = []
--> 534         self.loadFile(files)
    535 
    536     def loadFile(self, files):

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/engines/smirnoff/forcefield.py in loadFile(self, files)
    579 
    580         # Parse XML, get force definitions
--> 581         self.parseXMLTrees()
    582 
    583     def parseXMLTrees(self):

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/engines/smirnoff/forcefield.py in parseXMLTrees(self)
    626             for child in root:
    627                 if child.tag in parsers:
--> 628                     parsers[child.tag](child, self)
    629 
    630 

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/engines/smirnoff/forcefield.py in parseElement(element, ff)
   1781         # Register all SMIRNOFF torsion definitions.
   1782         for torsion in element.findall('Proper'):
-> 1783             generator.registerProperTorsion(torsion, element)
   1784         for torsion in element.findall('Improper'):
   1785             generator.registerImproperTorsion(torsion, element)

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/engines/smirnoff/forcefield.py in registerProperTorsion(self, node, parent)
   1761     def registerProperTorsion(self, node, parent):
   1762         """Register a SMIRNOFF torsiontype definition for a proper."""
-> 1763         torsion = PeriodicTorsionGenerator.ProperTorsionType(node, parent)
   1764         self._propertorsiontypes.append(torsion)
   1765 

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/engines/smirnoff/forcefield.py in __init__(self, node, parent)
   1680             # Check that the SMIRKS pattern matches the type it's supposed to
   1681             try:
-> 1682                 chemenv = ChemicalEnvironment(self.smirks)
   1683                 thistype = chemenv.getType()
   1684                 if thistype != 'Torsion':

~/data/applications/anaconda3/envs/smirnoff-paprika/lib/python3.6/site-packages/openforcefield/typing/chemistry/environment.py in __init__(self, smirks, label, replacements)
    468             except:
    469                 raise SMIRKSParsingError("Error SMIRKS (%s) was not parseable\
--> 470                         into a ChemicalEnvironment" % smirks)
    471 
    472         # Check that the created Environment is valid

SMIRKSParsingError: (SMIRKSParsingError(...), 'Error SMIRKS ([*:1]~[*:2]~[*:3]~[*:4]) was not parseable                        into a ChemicalEnvironment')
slochower commented 6 years ago

Weirdly, I was tying my conda environment to a particular commit and hence my usage of forcefield/smirnoff99Frosst.ffxml. But if I update to commit 32ad91a0fc20e167738810392a7d9452d4dda3bc and use forcefield/smirnoff99Frosst.offxml, things seem to work. I guess there's an import dependency tree that gets traversed that I don't quite understand. I've found it useful to test my notebooks in separate conda environments, created by having conda call pip to install a specific commit, like so:

name: smirnoff-paprika
channels:
  - anaconda
  - bioconda
  - conda-forge
  - omnia
  - openeye
dependencies:
  - python=3.6
  - anaconda::numpy=1.13.3
  - anaconda::scipy=1.0.0
  - anaconda::matplotlib=2.0.2
  - anaconda::ipykernel=4.8.0
  - anaconda::networkx=1.11
  - anaconda::lxml=3.8.0
  - anaconda::pylint
  - anaconda::pip
  - bioconda::nglview=1.0
  - omnia::parmed==2.7.3
  - omnia::openmm=7.1.1
  - omnia::openmoltools=0.8.1
  - openeye::openeye-toolkits
  - yapf
  - pip
  - pip:
    - git+https://github.com/open-forcefield-group/openforcefield@32ad91a0fc20e167738810392a7d9452d4dda3bc
    - git+https://github.com/slochower/smirnoff-host-guest@ea17852ca64127340baec2731ab7427ced8d9407
    - git+https://github.com/slochower/paprika/@62869c7bb86379f9dda44feb904fb8f2c8379432

...but I guess this shows that approach can have hidden issues.

slochower commented 6 years ago

I take back what I said about everything working. I'm sorry for not providing a full pipeline, but now I'm using the latest version of this repository and the offxml file.

SMIRKSParsingError: (SMIRKSParsingError(...), 'Error SMIRKS ([*:1]-[#6X4:2]-[#6X4:3]-[*:4]) was not parseable                        into a ChemicalEnvironment')

This seems unexpected to me. I'm reading in SYBYL format (yes!) mol2 files that were not giving an error previously. I've uploaded them here for reference -- but that such a common-seeming parameter is missing makes me think it's an issue with the force field file -- right? I can help diagnose further tomorrow.

MGO-sybyl.mol2.txt MOL-sybyl.mol2.txt

davidlmobley commented 6 years ago

@slochower - this sounds like it's a problem with parsing of the OFFXML file rather than a problem with your molecules. And specifically, it even seems like it might be a problem WITH YOUR ChemicalEnvironments class, as those look like valid SMIRKS patterns to me. Have you been making changes to the code? It would be easiest for me to understand this message if I thought you had broken the ChemicalEnvironments class in your openforcefield installation.

Can you provide the exact OFFXML file you are using?

slochower commented 6 years ago

I actually have not been messing with ChemicalEnvironments, although the error may still be on my end. I apologize for making work without finding the most minimal example where the issue occurs. It may be a problem with having openforcefield installed in multiple conda environments (I found it helpful to pin a certain version of openforcefield while working on the conversion functions to eliminate one potentially changing variable). I was under the impression they would be effectively "firewalled" from each other, but I'm starting to doubt that.

Here is the offxml file -- it looks like the one online (but I didn't sha1sum or antyhing). smirnoff99Frosst.offxml.txt

davidlmobley commented 6 years ago

Hmm, sounds odd. So I think the next step would be for me to try applying the FF to your molecules in your system, but I'm missing one file for that: Can you provide whatever file it is you're getting your Topology from? (It looks like you are saying you're processing two molecules at once, so...)

FWIW, parsing of this DOES get tested automatically on Travis-CI, and it works, so I suspect some kind of issue on your end.

You're right that the conda environments should in general be sandboxed from one another.

slochower commented 6 years ago

I am increasingly sure this is not your problem at all and something else local going wacky. I have the latest version of openforcefield in my current conda environment, but I'm loading other modules I wrote that simply call import openforcefield and I think import statement was pulling in an older version of openforcfield installed in the default (I think called root) conda environment. I surmise this is an issue with jupyter notebook kernel because I was not getting the problems running the scripts directly, which is why I perceived this to be a new issue and given that the first error popped up with the wildcard SMIRKS pattern, I suspected it might have had to do with this PR.

I have attached the topology file, but the caveat is that I split the topology (to eliminate waters and ions) before loading into createSystem, so the file isn't ready to go as-is. However, I don't believe it is worth your time to continue troubleshooting because I'm not sure I have all my ducks in a row over here. smirnoff.pruned.pdb.txt

davidlmobley commented 6 years ago

OK. Maybe check if your ducks are in a row. It's not obvious to me what old version you might have which would have this issue; on the other hand, I'm having trouble wrapping my head around how we could have something so wrong that it would lead to this problem, while also still passing tests on Travis.

(This is part of what Travis-CI is for -- it's basically automatically verifying, everytime we change anything, that everything that should run, DOES run and give correct results in a clean install on a virtual machine. So I can tell you that not only this works for me on my machine, but it works in a clean install on a new virtual machine, which strongly suggests it's a problem on your end. Also it's an argument for -- if you're a developer -- CI testing since you don't have to worry as much that you'll break something and only find out about it when your users complain.)

slochower commented 6 years ago

Yes, I agree 100% -- we have Travis running for some of our projects too, for precisely those reasons, but I don't usually use it with exploratory notebooks when it is not obvious what to test yet. I also agree that the error was perplexing! I am afraid that I've seemed to fix the issue but not know the cause. I basically removed the relevant conda environments and reinstalled them, shutdown all the notebook kernels, and so on. Maybe it was a "namespace" issue of some sort, where both old and new modules were in memory.

bannanc commented 6 years ago

@slochower It sounds like this was an environment problem, can we close this issue, or do you still need help working it out?

slochower commented 6 years ago

Close!

FWIW, in case anyone else stumbles upon this, I believe I know the issue. I was updating my conda environment and then restarting the kernel in my jupyter notebook to re-run stuff. I could see that new versions were being parsed by checking __version__ strings but something was lurking in memory causing problems (this may be why the new version of openforcefield was looking for some old wildcard patterns). Completely killing the jupyter process and then re-launching got rid of the errors.

bannanc commented 6 years ago

We removed generics back in PR #74 and then I forgot to close this after the discussion above.