Automatic unit compatibility checking

[kyleabeauchamp]

timestep = 2.0 / u.picoseconds
integrator = mm.VerletIntegrator(timestep)

In [45]: timestep
Out[45]: Quantity(value=2.0, unit=/picosecond)

In [46]: integrator.getStepSize()
Out[46]: Quantity(value=2.0, unit=picosecond)

To me, this looks like the unit conversion is trying to be smart and automatically invert per-picoseconds into picoseconds. However, it seems to be failing.

IMHO, this sort of automatic inversion should not even be attempted without a warning or error.

Thoughts?

[jchodera]

I agree completely. This should be a bug.

If units are specified, I don't think incompatible units should be allowed---an Exception should be thrown.

There may be a simple way to implement unit checking for these functions using a decorator.

[peastman]

This is due to the way Randy originally implemented the Python wrappers. It doesn't actually know the expected units of any arguments. It just knows the standard OpenMM unit system. So it assumes whatever value you pass has the correct type of units, and converts it to a value in the corresponding standard unit.

I agree it would be better if it actually knew the expected units, but this would be a very large feature to implement.

[jchodera]

@peastman : I actually implemented a similar feature using decorators for my pure-Python version of OpenMM's System and Force classes a while back. This may be something where we can create a PR and implement it to be integrated back into the main branch.

@kyleabeauchamp : Let's discuss when I get back. In the meantime, I'll send you an example of what I did previously.

[jchodera]

@kyleabeauchamp : See pyopenmm.py in the yank/yank/ directory, specifically the accepts_compatible_units decorator:

def accepts_compatible_units(*units):
    """
    Decorator for class methods that should accept only arguments compatible with specified units.

    Each argument of the function will be matched with an argument of @acceptunits.
    Those arguments of the function that correspond @acceptunits which are not None
    will be checked to ensure they are compatible with the specified units.

    EXAMPLE

    @acceptsunits(units.meter, None, units.kilocalories_per_mole)
    def function(a, b, c): pass
    function(1.0 * units.angstrom, 3, 1.0 * units.kilojoules_per_mole)

    """
    def check_units(f):
        nargs = (f.func_code.co_argcount - 1) # exclude self
        assert len(units) == nargs, "incorrect number of units supplied in @accepts_compatible_units decorator for class method %s" % (f.func_name)
        def new_f(*args, **kwds):
            for (a, u) in zip(args[1:], units):
                if u is not None:                    
                    assert (a.unit).is_compatible(u), "arg %r does not have units compatible with %s" % (a,u)
            return f(*args, **kwds)
        new_f.func_name = f.func_name # copy function name
        new_f.func_doc = f.func_doc # copy docstring
        return new_f
    return check_units

This may not be the best way to do it, but it may give us some ideas.

[jchodera]

Let's mark this as a potential "enhancement". We can try the accepts_compatible_units unit checking soon.

[jchodera]

Maybe rename the issue "Automatic unit compatibility checking"?

[peastman]

This is fixed by #3010.