astropy: Incorrect/inconsistent equality of custom units (value rounding error)

Description

>>> import astropy.units as u
>>> Hz = u.def_unit('Hz', u.cycle / u.s)
>>> kHz = u.def_unit('kHz', 1000 * Hz)
>>> 1 * kHz == 1000 * Hz
True
>>> 1000 * Hz == 1 * kHz
False

Of course, both equalities should return True, and in general equalities should be commutative.

(As an aside, I understand why Astropy defines Hz as 1/s, but in my field of work, cycle/s is more convenient and less error-prone, so I want to be able to redefine it.)

System Details

This was found on an Anaconda install on Windows:

  • Windows-10-10.0.19042-SP0
  • Numpy 1.20.3
  • pyerfa 2.0.0
  • astropy 5.0
  • Scipy 1.7.1
  • Matplotlib 3.5.0

About this issue

  • Original URL
  • State: open
  • Created 3 years ago
  • Comments: 16 (14 by maintainers)

Most upvoted comments

If the quantities have integer values (or float values that represent integer value), then it would be nice to have exact conversions though, whenever possible.

But I agree; we should keep the code as is, because it would require more thought to do this right than just hacking the _to() function. E.g. it might be worthwhile to try to also getting the Hz thing right.

Nevertheless, I thought of several ways to accomplish exact conversion, for my immediate goal of mm <-> um <-> nm. The (enormous) kludge that I settled on, makes use of the fact that SI unit conversions have short human readable representations by design. I’m only showing it here because I promised, and just maybe it could help someone:

def save_unit_to(unit_from: u.Unit, unit_to: u.Unit):
    """Like unit_from.to(unit_to), but u.um to u.nm == 1000., not 999.999.."""
    to1 = unit_from.to(unit_to)
    sto1 = str(to1)
    to2 = 1 / unit_to.to(unit_from)
    sto2 = str(to2)
    if len(sto2) < len(sto1):
        return to2
    return to1

with as test:

def test_save_to():
    for ufrom, uto, fac in [
        (u.um, u.nm, 1000.),
        (u.mm, u.um, 1000.),
        (u.nm, u.um, 0.001),
        (u.um, u.mm, 0.001),
    ]:
        assert save_unit_to(ufrom, uto) == fac

Maybe someone will say “Noooooooo…” and implement it properly.