pint icon indicating copy to clipboard operation
pint copied to clipboard

Converting a Decimal number results in a loss of precision

Open cdwilson opened this issue 1 year ago • 3 comments

Discussed in https://github.com/hgrecco/pint/discussions/2105

Converting a Decimal number from inch to thou results in a loss of precision. I would expect that this conversion does the equivalent of Decimal("1.0") * 1000, i.e. the conversion returns Decimal('1000.0').

Originally posted by cdwilson December 31, 2024

>>> from decimal import Decimal
>>> from pint import UnitRegistry
>>> ureg = UnitRegistry(non_int_type=Decimal)
>>> d2 = ureg.Quantity(Decimal("1.0"), ureg.inch)
>>> d2.to(ureg.thou).magnitude
Decimal('999.9999999999999999999999996')

cdwilson avatar Jan 01 '25 02:01 cdwilson

Hi

I experience the same issue with different units.

import decimal, pint
ureg = pint.UnitRegistry(non_int_type=decimal.Decimal)
flow = decimal.Decimal('2')*ureg.L/ureg.h
print(flow.to(ureg.L/ureg.day)) 

gives 48.00000000000000000000000002 liter / day not 48 liter / day.

I read through https://github.com/hgrecco/pint/issues/967 and assumed this was fixed. Maybe this is a new issue?

nneskildsf avatar Jan 02 '25 10:01 nneskildsf

The trouble appears to have it's roots in fractions. For my own case the conversion required computation of 1/60/60 which cannot be represented exactly as a decimal no matter the precision.

decimal actually has facilities to circumvent this. However, pint uses exponentiation to divide which causes decimal to take the non-clever path.

To fix this the following change is required to facets/plain/registry.py:

    def _get_root_units_recurse(
        self, ref: UnitsContainer, exp: Scalar, accumulators: dict[str | None, int]
    ) -> None:
        """

        accumulators None keeps the scalar prefactor not associated with a specific unit.

        """
        for key in ref:
            exp2 = exp * ref[key]
            key = self.get_name(key)
            reg = self._units[key]
            if reg.is_base:
                accumulators[key] += exp2
            else:
                # Use division operator for division
                if exp2 < 0:
                    accumulators[None] /= reg.converter.scale**abs(exp2)
                else:
                    accumulators[None] *= reg.converter.scale**abs(exp2)
                if reg.reference is not None:
                    self._get_root_units_recurse(reg.reference, exp2, accumulators)

With this change

import decimal, pint
ureg = pint.UnitRegistry(non_int_type=decimal.Decimal)
flow = decimal.Decimal('2')*ureg.L/ureg.h
print(flow.to(ureg.L/ureg.day).magnitude)

gives 48.00000000000000000000000000.

nneskildsf avatar Jan 02 '25 12:01 nneskildsf

Evidently there are additional problems for your example, @cdwilson.

To do the conversion from inch to thou pint performs the following calculation with my patch listed above:

thou = Decimal('0.02777777777777777777777777778') * Decimal('0.9144') / Decimal('0.001') / Decimal('0.02777777777777777777777777778') / Decimal('0.9144')
print(thou)

which returns 999.9999999999999999999999999. This is still not 1000, but it appears that this is the best result possible given the convoluted calculation path.

I certainly think it could be really interesting to adapt _get_root_units_recurse to cancel out terms that appear in numerator and denominator to simplify the calculation and to arrive at the correct result.

Updated: PR ready: https://github.com/hgrecco/pint/pull/2108 I have tried our examples and they both check out - even without using decimal! I suggest you try it out: pip install git+https://github.com/nneskildsf/pint.git@fix-decimal-loss-of-precision

nneskildsf avatar Jan 02 '25 13:01 nneskildsf