Plasmapy: Flag to ignore units

[lemmatum]

Just having gone through this with scipy.integrate.tplquad for integrating over distribution functions, it might be handy to implement a similar flag for all functions where we wish to ignore the checking and converting of units in certain cases. This would, in effect, be an optimization flag for speeding up code that doesn't play well with all the unit calls.

Opinions?

[lemmatum]

Relates to #59

[StanczakDominik]

Those performance issues are exactly what I was afraid of when I was initially looking into the units package...

I'm not sure this needs to be optional. There's going to be code where units are an issue
(like your tplquad integration case), and then there's going to be code where adding units doesn't matter much - most of physics.parameters comes to mind. There's not much point in having a flag to disable unit checks for a function consisting of three multiplications and a sqrt.

How about this as a general framework for functions:

1. Get inputs, check units
2. If this is a computationally intensive function, drop units
3. Do the calculations quickly
4. If units were dropped, add them back on the way out

Of course we'd need to have that code tested properly and slowly the first time it's added to make sure we're not making any mistakes ourselves.

Does that make sense?

[lemmatum]

In some cases, like tplquad, it won't work because the inputs and outputs have to be unitless. So you just end up with an entirely unitless function. Also, I don't know how much dropping units within the function saves you on time. My guess is that this will heavily depend on what this function is being used for. If you're running something which is essentially a thin-wrapper over some highly efficient C code, then I assume passing around these astropy objects will kill many of the optimizations... really depends on how close astropy is to numpy I think.

[Cadair]

Dropping units is basically free. Converting units is obviously not. Stripping units and then putting them back will take some time but should not be as bad as some weird C layer side effects of having a ndarray subclass.

[lemmatum]

@Cadair on principle I agree with you, but has this been tested?

[StanczakDominik]

@Cadair wrote a good part of astropy.units, so I'd be willing to trust him on this one. :smile:

[lemmatum]

fair play 😁 . I'm still going to plead ignorance and ask if this has been tested. Sometimes one finds surprises 😜

[antoinetavant]

With a simple notebook, we can see that droping units for a simple multiplication can increase the performance by 2...

We could start with the framework sugjected by @StanczakDominik , and if the performence really is an issue add a flag...

There is the notebook:

import astropy.units as u
a = 1 * u.m
b = 2* u.s

%%timeit
c = a * b

56.4 µs ± 1.01 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

%%timeit

aprim = a.value
bprim = b.value

c = aprim*bprim

c *= u.m*u.s

28.7 µs ± 165 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)

[Cadair]

@Cadair wrote a good part of astropy.units

This is wildly overstating the facts 😄 I contributed u.quantity_input.

My general feeling is some performance cost is worth units, but performance sensitive code should be unitless where appropriate. Just make sure to balance that with the maintenance burden of the extra complexity.