openff-toolkit icon indicating copy to clipboard operation
openff-toolkit copied to clipboard

VirtualSiteType.inPlaneAngle and outOfPlaneAngle._unit is None

Open lilyminium opened this issue 2 years ago • 2 comments
trafficstars

Describe the bug

Despite the code appearing to do otherwise, both VirtualSiteType.inPlaneAngle._unit and VirtualSiteType.outOfPlaneAngle._unit is None. Other unit-ed attributes, like VirtualSiteType.distance, don't have this issue. I'm not sure if this is actually a bug but I am a bit surprised. Relevant to #1637

Code here: https://github.com/openforcefield/openff-toolkit/blob/a53a7e1b29d6ec134114da517b17d19b34908602/openff/toolkit/typing/engines/smirnoff/parameters.py#L3368-L3370

To Reproduce

In [1]: import openff.toolkit

In [2]: openff.toolkit.__version__
Out[2]: '0.13.1+3.ga53a7e1b'

In [3]: ff = openff.toolkit.ForceField()

In [4]: handler = ff.get_parameter_handler("VirtualSites")

In [5]: handler.VirtualSiteType.inPlaneAngle._unit

In [6]: assert handler.VirtualSiteType.inPlaneAngle._unit is None

In [7]: assert handler.VirtualSiteType.outOfPlaneAngle._unit is not None
---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
Cell In[7], line 1
----> 1 assert handler.VirtualSiteType.outOfPlaneAngle._unit is not None

AssertionError:

In [8]: assert handler.VirtualSiteType.distance._unit is not None

Output

Computing environment (please complete the following information):

  • Operating system
  • Output of running conda list

Additional context

lilyminium avatar Jun 13 '23 04:06 lilyminium

This is weird. There must be some special logic that is gunking this up, since there's nothing obvious about the base class or unit.

In [8]: ParameterAttribute(unit=unit.degree)._unit
Out[8]: <Unit('degree')>

In [9]: ParameterAttribute(unit=unit.degrees)._unit
Out[9]: <Unit('degree')>

In [10]: unit.degree.is_compatible_with(unit.degrees)
Out[10]: True

In [11]: unit.degree.dimensionality, unit.degrees.dimensionality
Out[11]: (<UnitsContainer({})>, <UnitsContainer({})>)

mattwthompson avatar Jun 13 '23 14:06 mattwthompson

I'm super stumped by this. I tried

  • fiddling with the converters, since they're one of the only way that these differ from VirtualSiteType.distance, which doesn't exhibit this behavior
  • unit.degree vs. unit.degrees makes no difference, not that one would expect it to
  • There might be something happening in _add_default_init_kwargs but this behavior can be reproduced without actually creating instances of VirtualSiteHandler etc.

These keywords aren't even hit often in other places in the code, so I'm worried there's something spooky happening deeper in the ParameterAttribute logic. These classes are hard for me to understand top-to-bottom, and I worry that changing anything will break everything, so I didn't explore that much.

mattwthompson avatar Jun 13 '23 15:06 mattwthompson