Use astropy.units more consistently across POPPY

[mperrin]

Issue by douglase Thursday Jan 28, 2016 at 17:09 GMT Originally opened as https://github.com/mperrin/poppy/issues/145

---

As discussed this morning it would be useful to automatically handle datasets with units other than meters. @josePhoenix the test for units that keeps things backwards compatible in fresnel.py is (https://github.com/mperrin/poppy/blob/master/poppy/fresnel.py#L50-L54):

    if isinstance(z, u.quantity.Quantity):
        self.z_m = z.to(u.m)  # convert to meters.
    else:
        _log.debug("Assuming meters, phase {:.3g} has no units for Optic: " .format(z) + self.name)
        self.z_m = z * u.m

I think we would want to make it a general function, since it's reused several place. If there's an easy way to enforce it with a decorator that sounds like it could be a more elegant solution.

[mperrin]

Comment by mperrin Thursday Jan 28, 2016 at 17:20 GMT

---

See here for the astropy QuantityInput decorator source: http://docs.astropy.org/en/stable/_modules/astropy/units/decorators.html

This is different from what you suggest - because it raises a TypeError if the argument lacks a unit, instead of applying a default one - but the code could easily be modified to behave as you describe.

Mock code:

    @back_compatible_quantity_input(radius=u.m)
    def some_function(self, radius=1.0*u.m, more_args)
         code...

And it would apply units of meters and print that log debug message if the parameter lacks units, or raise an error if it has incompatible units, or just pass through the argument if given something with units of length.

[mperrin]

Comment by mperrin Thursday Apr 21, 2016 at 21:29 GMT

---

I now have a working version of a decorator with the desired functionality. (It may seem like "a more elegant solution" on the outside; inside it's dark magic with Python metaprogramming that I kludged up based on the astropy decorator example).

Right now I've applied that decorator to the Wavefront constructor so that the wavelength and diam parameters can be provided as astropy Quantity lengths (in , or just as floating point values, in which case they are automatically turned into Quantity lengths in meters. This basic part all seems to work, yay.

Of course, now a huge fraction of the unit tests fail because the rest of the code chokes on having Quantities...

This is in a private branch for now but I could post it here if others are interested in working on it and trying to get the tests passing. Let me know if interested.

[mperrin]

Comment by mperrin Monday Apr 25, 2016 at 22:04 GMT

---

Much progress - this is nearing a state where I could merge it, with all tests passing.

But it's a slippery slope: once you add units support for wavelengths, it's temping to add it for all the pupil radii and other distances defined in all the optics. And then what about the units on the OPD arrays themselves? And so forth. It's actually somewhat problematic to only go part way on this, because then various combinations that are supposed to cancel out don't end up being dimensionless, and that breaks all the trig function arguments etc.

Not sure how far it's worth taking this - I'm tempted to say what I have thus far with all wavelengths allowed to have units is good enough, and not apply dimensional quantities self-consistently throughout all the optics.