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

ensure_quantity should attempt to wrap arrays and primitives #45

Open
mattwthompson opened this issue Sep 19, 2022 · 0 comments
Open

ensure_quantity should attempt to wrap arrays and primitives #45

mattwthompson opened this issue Sep 19, 2022 · 0 comments

Comments

@mattwthompson
Copy link
Member

Here's a rough edge I found while debugging some code in which I can't trust the type of the object I'm interacting with:

In [1]: from openff.units.openmm import ensure_quantity

In [2]: import numpy

In [3]: partial_charges = numpy.linspace(-1, 1, 9, dtype=float)

In [4]: ensure_quantity(partial_charges, "openff")
---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)
Cell In [4], line 1
----> 1 ensure_quantity(partial_charges, "openff")

File ~/mambaforge/envs/openmmforcefields-test/lib/python3.9/site-packages/openff/units/openmm.py:240, in ensure_quantity(unknown_quantity, type_to_ensure)
    238     return _ensure_openmm_quantity(unknown_quantity)
    239 elif type_to_ensure == "openff":
--> 240     return _ensure_openff_quantity(unknown_quantity)
    241 else:
    242     raise ValueError(
    243         f"Unsupported `type_to_ensure` found. Given {type_to_ensure}, "
    244         "expected 'openff' or 'openmm'."
    245     )

File ~/mambaforge/envs/openmmforcefields-test/lib/python3.9/site-packages/openff/units/openmm.py:230, in _ensure_openff_quantity(unknown_quantity)
    226         raise ValueError(
    227             f"Failed to process input of type {type(unknown_quantity)}."
    228         )
    229 else:
--> 230     raise Exception

Exception:

In #44 I only considered that openmm.unit.Quantity or openff.units.Quantity would be passed through. I think some users would expect ensure_quantity to make some effort to coerce things into those types if asked. I'm less passionate about coercing int/float but the behavior should be similar.

The main downside of this approach is that it the resulting object must be a unitless quantity. Pint handles explicitly unitless quantities in a way that feels naturally to me via carrying around a dimensioness unit. OpenMM does a pretty good job with a similar solution but has a couple of gotchas, i.e. openmm/openmm#2247:

In [1]: from openff.units import unit

In [2]: unit.Quantity(1) / unit.Quantity(2)
Out[2]: 0.5 <Unit('dimensionless')>

In [3]: from openmm import unit

In [4]: unit.Quantity(1) / unit.Quantity(2)
Out[4]: 0.5
@mattwthompson mattwthompson changed the title ensure_quantity should wrap arrays if provided ensure_quantity should attempt to wrap arrays and primitives Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant