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

Can't pickle ValidatedList #411

Closed
trevorgokey opened this issue Sep 3, 2019 · 3 comments · Fixed by #425
Closed

Can't pickle ValidatedList #411

trevorgokey opened this issue Sep 3, 2019 · 3 comments · Fixed by #425

Comments

@trevorgokey
Copy link
Collaborator

trevorgokey commented Sep 3, 2019

Trying to unpickle anything with a ValidatedList fails:

from openforcefield.typing.engines.smirnoff.forcefield import ForceField 
import smirnoff99frosst as ff
import pickle
import os
abspath = os.path.join( ff.get_forcefield_dirs_paths()[0], \
                        'smirnoff99Frosst-1.1.0.offxml')
forcefield= ForceField( abspath)

with open("out.p", 'wb') as fid:
    pickle.dump( forcefield, fid)
with open("out.p", 'rb') as fid:
    x = pickle.load( fid)

Out:

Traceback (most recent call last):
  File "bug.py", line 12, in <module>
    x = pickle.load( fid)
  File "/home/tgokey/.local/miniconda3/envs/oFF/lib/python3.7/site-packages/openforcefield/utils/collections.py", line 106, in extend
    iterable = self._convert_and_validate(iterable)
  File "/home/tgokey/.local/miniconda3/envs/oFF/lib/python3.7/site-packages/openforcefield/utils/collections.py", line 141, in _convert_and_validate
    if self._converters is not None:
AttributeError: 'ValidatedList' object has no attribute '_converters'

My own debugging shows that these lists are likely being pickled as upcasted lists. Might need to implement a __getstate__ and __setstate__ to make sure the appropriate members are persisted.

@trevorgokey
Copy link
Collaborator Author

trevorgokey commented Sep 7, 2019

So I figured it out. For whatever reason, __setstate__ never got called during unpickling, so the above idea didn't work. The following method works (for pickling and unpickling):

    def __reduce__( self):      
        return (__class__, ( list( self),), self.__dict__)

You should be able to drop this into anything that subclass list and et viola allows pickling. See https://docs.python.org/3/library/pickle.html#object.__reduce__ for documentation.

I was able to pickle an entire ForceField object with this function added to ValidatedList. Here is what I see from an unpickled forcefield.__dict__:

In [37]: forcefield.__dict__
Out[37]: 
{'_MIN_SUPPORTED_SMIRNOFF_VERSION': 0.1,
 '_MAX_SUPPORTED_SMIRNOFF_VERSION': 0.3,
 '_disable_version_check': False,
 '_aromaticity_model': 'OEAroModel_MDL',
 '_parameter_handler_classes': OrderedDict([('Constraints',
               openforcefield.typing.engines.smirnoff.parameters.ConstraintHandler),
              ('Bonds',
               openforcefield.typing.engines.smirnoff.parameters.BondHandler),
              ('Angles',
               openforcefield.typing.engines.smirnoff.parameters.AngleHandler),
              ('ProperTorsions',
               openforcefield.typing.engines.smirnoff.parameters.ProperTorsionHandler),
              ('ImproperTorsions',
               openforcefield.typing.engines.smirnoff.parameters.ImproperTorsionHandler),
              ('vdW',
               openforcefield.typing.engines.smirnoff.parameters.vdWHandler),
              ('Electrostatics',
               openforcefield.typing.engines.smirnoff.parameters.ElectrostaticsHandler),
              ('ToolkitAM1BCC',
               openforcefield.typing.engines.smirnoff.parameters.ToolkitAM1BCCHandler),
              ('ChargeIncrementModel',
               openforcefield.typing.engines.smirnoff.parameters.ChargeIncrementModelHandler),
              ('GBSA',
               openforcefield.typing.engines.smirnoff.parameters.GBSAParameterHandler)]),
 '_parameter_handlers': OrderedDict([('Constraints',
               <openforcefield.typing.engines.smirnoff.parameters.ConstraintHandler at 0x7f251a1f2978>),
              ('Bonds',
               <openforcefield.typing.engines.smirnoff.parameters.BondHandler at 0x7f251a1f2a90>),
              ('Angles',
               <openforcefield.typing.engines.smirnoff.parameters.AngleHandler at 0x7f251a27a5c0>),
              ('ProperTorsions',
               <openforcefield.typing.engines.smirnoff.parameters.ProperTorsionHandler at 0x7f251a280940>),
              ('ImproperTorsions',
               <openforcefield.typing.engines.smirnoff.parameters.ImproperTorsionHandler at 0x7f251a07cda0>),
              ('vdW',
               <openforcefield.typing.engines.smirnoff.parameters.vdWHandler at 0x7f251a07c1d0>),
              ('Electrostatics',
               <openforcefield.typing.engines.smirnoff.parameters.ElectrostaticsHandler at 0x7f25184ef160>),
              ('ToolkitAM1BCC',
               <openforcefield.typing.engines.smirnoff.parameters.ToolkitAM1BCCHandler at 0x7f25184efb70>)]),
 '_parameter_io_handler_classes': OrderedDict([('XML',
               openforcefield.typing.engines.smirnoff.io.XMLParameterIOHandler)]),
 '_parameter_io_handlers': OrderedDict([('XML',
               <openforcefield.typing.engines.smirnoff.io.XMLParameterIOHandler at 0x7f25184ef630>)]),
 '_parameters': [],
 '_author': 'C. I. Bayly, OpenEye/UC Irvine; C. C. Bannan, UC Irvine; D. L. Mobley, UC Irvine; J. R. Wagner, Open Force Field/UC Irvine',                                                                                          
 '_date': 'Date: July 25, 2019',
 'disable_version_check': True}

The only thing empty is _parameters: not sure if that is intentional.

@j-wags j-wags added this to the 0.5.1 milestone Sep 13, 2019
@j-wags
Copy link
Member

j-wags commented Sep 13, 2019

Very cool. Thanks for the solution! I've put this into the 0.5.1 milestone, and will tie it in as soon as I start development towards that!

@j-wags
Copy link
Member

j-wags commented Sep 20, 2019

I don't think we actually want ForceField._parameters. Only ParameterHandlers should have that. I'm going to try removing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants