Repository navigation
Fix Quantity * Quantity and / Quantity on compound ratio units - #5478
Conversation
Quantity.__mul__ delegated quantity*quantity to (quantity*scalar)*unit and __truediv__ mirrored it. When self's unit is a dimensionless compound ratio such as nanometer/angstrom, the scalar operation collapses it to a bare value first, so the later unit operation reattached only the other quantity's unit. Products dropped a unit factor (3.0 nm/A squared gave "9.0 nm/A" instead of 900.0) and quotients inherited the wrong unit entirely (a/a gave 0.1, a/b gave 15.0 instead of 150.0). Multiply/divide the raw values and combine the units in one Quantity, then reduce_unit as before. This preserves the existing float association for ordinary units while fixing the ratio-unit collapse. Fixes openmm#5445.
|
Looks good. With the changes from #5477, however, your test case no longer tests what you intend since |
Since openmm#5477, scalar * dimensionless-unit expressions evaluate to plain floats, so 3.0 * (u.nanometer / u.angstrom) no longer produces a Quantity and the test stopped exercising Quantity arithmetic. Build the test values with u.Quantity() directly.
|
Updated in 0c19ede. The test now builds the values with |
|
Tests are failing: |
The raw value*value and value/value paths raise TypeError when either Quantity wraps a plain list or tuple (e.g. Vec3 values). Fall back to _scale_sequence in that case, matching _change_units_with_factor. Generated with [Devin](https://devin.ai)
|
Fixed in 23c98db. The raw
|
|
Thanks! |
Summary
Fixes #5445.
Quantity.__mul__delegatedQuantity * Quantityto(Quantity * scalar) * unit, and__truediv__mirrored it. Whenself's unit is a dimensionless compound ratio such asnanometer/angstrom, the scalar half collapses the unit to dimensionless and returns a bare value, so the following unit operation sees only the other operand's unit:The fix multiplies or divides the raw values and combines both units in a single
Quantitybeforereduce_unit. Operating on raw values also preserves the previous float association for ordinary units, so1.3 m / 75.2 cmstill produces the bit-identical resultTestUnits.testQuantityMathsasserts.Testing
Added
testCompoundRatioMultiplyDividetoTestUnits.pycovering multiply and divide on ratio units (including self-similar, inverse, and mixed-scale ratios). A non-dimensionless sanity check confirms ordinary products are unaffected. All 55 tests inTestUnits.pypass.Checklist
Generated by Devin