amusecode
Attempt at replacing | with * for creating quantities
See issue #687. This PR makes it possible to construct quantities using the * operator, which would previously have resulted in a unit.
With this PR, a unit multiplied/divided by another unit will still give a unit, while other multiplications/divisions will result in a quantity. Of course, quantities can still be converted to units with .to_unit().
Using the | operator to create quantities is still possible with this PR by the way - but if merged, we should add a deprecation warning to that method.
I think the impact should be minimal with this PR - only scripts that construct new units will need modification, and I don't recall ever writing such a script myself.
its very hard to review with the zillion formatting changes... [its not a good sign when github refuses to load the diff]
Lots of errors so far, working on fixing them. One problem is that the order of constructing units/quantities needs to be taken into account. Not sure what we can do about that except advising that any unit should be constructed first before assigning it to a value, using parentheses or something.
can you give an example of that?
what happens if you keep the old overload available?
it's still available, but some constructed units are now suddenly quantities. Leads to errors like this:
In [4]: (constants.Rydberg_constant * constants.h * constants.c)
...:
...:
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-4-4f3dad978eb6> in <module>
----> 1 (constants.Rydberg_constant * constants.h * constants.c)
2
~/Code/amuse/src/amuse/units/quantities.py in __mul__(self, other)
125 def __mul__(self, other):
126 other = to_quantity(other)
--> 127 return new_quantity_nonone(self.number * other.number, (self.unit * other.unit).to_simple_form())
128
129 __rmul__ = __mul__
~/Code/amuse/src/amuse/units/quantities.py in __mul__(self, other)
125 def __mul__(self, other):
126 other = to_quantity(other)
--> 127 return new_quantity_nonone(self.number * other.number, (self.unit * other.unit).to_simple_form())
128
129 __rmul__ = __mul__
~/Code/amuse/src/amuse/units/quantities.py in new_quantity_nonone(value, unit)
1215 :returns: new ScalarQuantity or VectorQuantity object
1216 """
-> 1217 if not unit.base:
1218 if isinstance(value, __array_like):
1219 return numpy.asarray(value) * unit.factor
AttributeError: 'ScalarQuantity' object has no attribute 'base'
its very hard to review with the zillion formatting changes... [its not a good sign when github refuses to load the diff]
should be a bit easier now. Main changes are in core.py.
can you give an example of that?
c = 299792458.0 * (m*s**-1) is fine, but c = 299792458.0 * m*s**-1 is not:
In [7]: c = 299792458.0 * m*s**-1
In [8]: c
Out[8]: quantity<299792458.0 s**-1 m>
In [9]: c.unit
Out[9]: unit<m>
In [10]: c.number
Out[10]: quantity<299792458.0 s**-1>
In [11]: c = 299792458.0 * (m*s**-1)
In [12]: c
Out[12]: quantity<299792458.0 m * s**-1>
In [13]: c.unit
Out[13]: unit<m * s**-1>
In [14]: c.number
Out[14]: 299792458.0
does the ror still work after this?
maybe is also good to take stock of all the implications that this change has (issue?):
- old simulation scipts
- documentation
- book
- sandbox
- downstream (dependend packages ie omuse)
- etc
Yes, the ror is untouched and still works.
maybe is also good to take stock of all the implications that this change has (issue?): old simulation scipts documentation book sandbox downstream (dependend packages ie omuse) etc
Sure. There must not be any remaining issues when this is merged*, so we'll need to test this extensively.
*: other than the creation of new units, which can by design not be done in the old way with this change...
quantities has a as_unit and a to_unit - they are slightly different...don't know why.. (i prefer the name to_unit, don't know about implementation yet)
also - there are a bunch of as_units that can be removed and I think in the constants definition we can also remove '* none' units
Another issue: 1.0 * unit will return the unit. This should probably return a quantity instead (in line with previous behaviour and other unit systems)
Another issue: 1.0 * unit will return the unit. This should probably return a quantity instead (in line with previous behaviour and other unit systems)
units.kg.__mul__(1.0) does return a quantity. Apparently with 1.0 * units.kg this isn't even called.
This also fails:
In [31]: 2.0 * 2.0 * kg * m**2
Out[31]: quantity<4.0 m**2 kg>
In [32]: 2.0 * 2.0 * kg**2 * m**2
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-32-7f60638bbeb4> in <module>
----> 1 2.0 * 2.0 * kg**2 * m**2
~/Code/amuse/src/amuse/units/quantities.py in __mul__(self, other)
125 def __mul__(self, other):
126 other = to_quantity(other)
--> 127 return new_quantity_nonone(self.number * other.number, (self.unit * other.unit).to_simple_form())
128
129 __rmul__ = __mul__
~/Code/amuse/src/amuse/units/quantities.py in new_quantity_nonone(value, unit)
1214 :returns: new ScalarQuantity or VectorQuantity object
1215 """
-> 1216 if not unit.base:
1217 if isinstance(value, __array_like):
1218 return numpy.asarray(value) * unit.factor
AttributeError: 'ScalarQuantity' object has no attribute 'base'
also:
In [33]: (2.0 * 2.0 * kg * m**2).unit
Out[33]: unit<kg>
In [34]: (2.0 * 2.0 * kg * m**2).number
Out[34]: quantity<4.0 m**2>
This is what's happening internally:
In [2]: 1.0 * (kg * m)
mul (kg, m) # self, other
rmul (kg * m, 1.0) # self, other
Out[2]: quantity<1.0 kg * m>
In [3]: 1.0 * kg * m
rmul (kg, 1.0)
rmul (m, 1.0)
mul (kg, none)
Out[3]: quantity<1.0 m kg>
And
In [4]: 1.0 * kg**2 * m
rmul (kg**2, 1.0)
rmul (m, 1.0)
mul (kg**2, none)
rmul (kg**2, 1)
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)```
In astropy:
In [9]: 1.0 * u.kg**2 * u.m
rmul kg2 1.0
mul m kg2
Out[9]: <Quantity 1.0 kg2 m>```
Yes, changing the order seems to work:
In [12]: 3.206361533e-53 * C**3*m**3*J**-2
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-12-545b55826c72> in <module>
----> 1 3.206361533e-53 * C**3*m**3*J**-2
~/Code/amuse/src/amuse/units/quantities.py in __mul__(self, other)
125 def __mul__(self, other):
126 other = to_quantity(other)
--> 127 return new_quantity_nonone(self.number * other.number, (self.unit * other.unit).to_simple_form())
128
129 __rmul__ = __mul__
~/Code/amuse/src/amuse/units/core.py in to_simple_form(self)
211 result = result * base
212 else:
--> 213 result = result * (base ** n)
214
215 return result
~/Code/amuse/src/amuse/units/quantities.py in __mul__(self, other)
125 def __mul__(self, other):
126 other = to_quantity(other)
--> 127 return new_quantity_nonone(self.number * other.number, (self.unit * other.unit).to_simple_form())
128
129 __rmul__ = __mul__
~/Code/amuse/src/amuse/units/quantities.py in new_quantity_nonone(value, unit)
1214 :returns: new ScalarQuantity or VectorQuantity object
1215 """
-> 1216 if not unit.base:
1217 if isinstance(value, __array_like):
1218 return numpy.asarray(value) * unit.factor
AttributeError: 'ScalarQuantity' object has no attribute 'base'
In [13]: C**3*m**3*J**-2 * 3.206361533e-53
Out[13]: quantity<3.206361533e-53 C**3 * m**3 * J**-2>
also the check for a numerical factor of exactly one should then maybe go to __mul__ or do something with factor_unit, but that is tricky...
did you see the comment on line 87 of core? (rmul)
yes, I think it's fixed now. Error is gone at least.
edit: no, it's not
~~can you make a version w/o formatting changes? its very hard to see where it stands now~~ its not so bad actually, though its a bit annoying to combine formatting with nonformatting changes...in any case have you looked at this further?