modulus of dimensioned quantities returns dimensionless quantity

[egradman]

In [10]: pq.Quantity(10.0,'m') % pq.Quantity(3.0, 'm')
Out[10]: array(1.0) * dimensionless

I would expect the remainder to be
array(1.0) * 'm'

When you divide two quantities, the quotient should be dimensionless but the remainder is dimensioned.

Also, "10m % 3" has no physical meaning, but pq returns "1m"

(thanks so much for your quick response yesterday. I was going around the office raving about this project all day! :)

[Darren Dale (dsdale24)]

I was confused about what modulus should do. Will try to fix by this weekend. Thanks for the encouraging remarks!

[Darren Dale (dsdale24)]

Here's a patch with the necessary changes, I'll submit to launchpad next time I log into linux:

p_dict[np.divide] = _d_divide
p_dict[np.true_divide] = _d_divide
+p_dict[np.floor_divide] = _d_divide
p_dict[np.remainder] = _d_divide

def _d_add_sub(q1, q2, out=None):
    try:
        return q1._dimensionality + q2._dimensionality
    except AttributeError:
        if hasattr(q1, 'dimensionality'):
            if np.asarray(q2).any():
                return q1._dimensionality + Dimensionality()
            else:
                return q1.dimensionality
        elif hasattr(q2, 'dimensionality'):
            if np.asarray(q1).any():
                return Dimensionality() + q2._dimensionality
            else:
                return q2.dimensionality
p_dict[np.add] = _d_add_sub
p_dict[np.subtract] = _d_add_sub
+p_dict[np.mod] = _d_add_sub

[Darren Dale (dsdale24)]

Just for my own edification, and to keep a record of the reasoning behind this bug: The discussion of modulo at wikipedia (http://en.wikipedia.org/wiki/Modulo_operation) lists the calculation as:

remainder = a-n[a/n]

where the quotient [a/n] is defined variously as truncated division, floor division, etc. depending on the convention. The right way to do this seems to be to convert n into units of a before calculating the remainder. Proper support for rescaling an input array would require an additional __input_prepare__ mechanism in numpy, which I proposed on the mailing list on 2009-10-17.

If a and n have different-but-compatible units, "a%n" and "a%=n" will work without __input_prepare__, since these are associated with special methods. Without __input_prepare__, remainder and fmod will not work unless they have identical units.

[Misha Dorman (mishad-work)]

I'm not sure I agree with the resolution of this bug -- specifically the second part:

"10m % 3" has no physical meaning, but pq (v0.5.x?) returns "1m"

10m % 3 has physical meaning just as 10m / 3 has meaning = 3.333m

Based on the formula

 remainder = a-n[a/n]

10m % 3 = 10m - 3*[10m/3] = 10m - 3*[3.3333m] = 10m - 3*(3m) = 10m - 9m - 1m

This appears to have worked correctly in earlier versions, but pq v0.7 on Python 2.6 now raises an exception:

>>> pq
<module 'quantities' from 'C:\Python26-i32-dev-venv\lib\site-packages\quantities-0.7.0-py2.6.egg\quantities\__init__.pyc'>
>>> 10*pq.m % 3
Traceback (innermost last):
  File "<stdin>", line 1, in <module>
  File "C:\Python26-i32-dev-venv\lib\site-packages\quantities-0.7.0-py2.6.egg\quantities\quantity.py", line 57, in g
    other = other.rescale(self.units)
  File "C:\Python26-i32-dev-venv\lib\site-packages\quantities-0.7.0-py2.6.egg\quantities\quantity.py", line 187, in rescale
    %(from_u._dimensionality, to_u._dimensionality)
ValueError: Unable to convert between units of "dimensionless" and "m"

I think this part of the change should be reverted, if its possible to do so without breaking the original fix. In fact, I think the original behaviour (works for % dimensionless, gives right answer but wrong dimension for % dimensioned) is more useful than the new behaviour (fails for % dimensionless, gives right answer and dimension/units for % dimensioned).

Misha

[Darren Dale (dsdale24)]

Hi Misha,

I take your point, but consider your example using different units:

foo = 10m % 3 = 10m - 3*[10m/3] = 10m - 3*[3.3333m] = 10m - 3*(3m) = 10m - 9m = 1m
bar = 10,000mm % 3 = 10,000mm - 3*[10,000mm/3] = 10,000mm - 3*[3,333.3333mm] = 10,000mm - 3*(3,333mm) = 1mm

fubar = foo != bar

I think this is why the original report claimed that "10m%3" has no *physical* meaning. I worry that if "10m%3" is allowed, it will be a frequent source of bugs. The fact that modulo returns a different physical quantity depending on the units of the operand lead me to conclude that mod should only be defined for dimensionless quantities. Based on that reasoning, I think I should probably change floor() and ceil() so they require dimensionless quantities. The whole point of the quantities package is that it should give you the same physical result regardless of what units are used. I'd like to hear your use case, but it would have to be very compelling to overturn the previous conclusions.

Darren