Unexpected behavior of unit conversions inside functions

[Mads-MMJ]

Description
When converting units inside a function it doesn't always behave as if done outside one. It seems that inside functions some units are simply not converted.
I have experienced this with the Angle dimension, specifically with the degree unit. It seems to me that it might have something to do with the Angle dimension being an alias for Scalar, but I have not yet tested it extensively enough to confirm this.

Example
In this example is the normal behavior outside a function:

>>> 12 deg -> 1 deg

  12 degree ➞ 1 degree

      = 12°
>>> trunc(12 deg -> 1 deg)

  trunc(12 degree ➞ 1 degree)

      = 12°

When the exact same code is moved into a function however, this is the result:

>>> fn test(a, b) = trunc(a -> b)

  fn test<A: Dim>(a: A, b: A) -> A = trunc(a ➞ b)

>>> test(12 deg, 1 deg)

  test(12 degree, 1 degree)

      = 0
>>> fn test_two(a, b) = a -> b

  fn test_two<A: Dim>(a: A, b: A) -> A = a ➞ b

>>> test_two(12 deg, 1 deg)

  test_two(12 degree, 1 degree)

      = 0.20944

The returned value is simply a scalar as if no conversion happened.
More importantly function calls inside the function work with the unconverted value as well giving results that conflict with those it would give outside the function for the same input.

[sharkdp]

Thank you for reporting this. I think you ran into #336.

The bottom line is this: Functions like trunc (but also round, floor, …) are inherently dangerous and we should really provide safe alternatives instead.

The problem is that trunc(quantity) will change the value when the unit of the quantity is changed, even if the overall quantity stays the same. For example:

let q1 = 123.4 cm
let q2 = q1 -> m  # the same as physical quantity as q1, just a different unit

trunc(q1)  # 123 cm
trunc(q2)  # 1 m = 100 cm

So we rather want something like your test/unit_trunc. I would probably call it trunc_in and reverse the arguments:

fn trunc_in<A: Dim>(unit_: A, value: A) -> A = trunc(value ➞ unit_)

This has sane behavior:

trunc_in(m,  q1)  # 1 m
trunc_in(m,  q2)  # 1 m

trunc_in(cm, q1)  # 123 cm
trunc_in(cm, q2)  # 123 cm

This also works for angles:

let alpha = 123.4 deg
trunc_in(deg, alpha)  # 123 deg
trunc_in(rad, alpha)  # 2 rad
trunc_in(mrad, alpha)  # 2153 mrad

What you discovered here is an interesting case though. The proposed trunc_in function does not work if we use 1 deg instead of deg in the first argument:

trunc_in(1 deg, alpha)  # 2 <-- WRONG

You discovered this in your implementation, because you used 1 deg instead of deg and/or value -> unit_of(unit_) instead of just value -> unit_ (where unit_of(deg) == 1 deg). The problem is that 1 deg = 1 × deg is an expression (instead of just a single unit), and Numbat always tries to simplify units after performing an operation like multiplication. In this case, it simplifies 1 × deg = 1 deg to 0.0174533 (see below).

The behavior is not really different in functions. It's rather the fact that you use an identifier on the right-hand side of ->. Consider:

>>> 12 ° -> 1 deg

    = 12°

>>> let x = 1 deg
>>> 12 ° -> x

    = 0.20944

The thing here is that angles in degree "decay" to scalars. I know that this is usually not the desired behavior and maybe that should be changed. When an explicit conversion is requested (12° -> deg), we keep the unit though. But that seems to rely on whether or not the conversion target is a plain value or an identifier. I currently don't know why.

The "decaying" comes from a unit-simplification feature of Numbat. We have a very simple heuristic that basically says: if the unit of a quantity can be converted to a scalar, then do so. This is very useful for more complex units, but not desirable for angle units.

So what can we do here to improve the situation?

• Implement trunc_in and friends. We could keep trunc as a Scalar -> Scalar function, and mention trunc_in in the docs for trunc.
• Somehow make sure that trunc_in can not be used with an expression as the unit_ argument. This is a tricky problem, as there is no way to express this in the type system right now. Alternatively:
• Re-think the simplification heuristic. Try to keep units like degree/arcmin but also radian around instead of decaying to a scalar.
• Look into the inconsistency above (value vs identifier on right hand side)

[sharkdp]

See #537 (comment) for an updated proposal that fixes two of the issues above.

[sharkdp]

We now provide safe versions of trunc: trunc_in. See #546 for details.

See also #537 which will probably fix the inconsistency with unit conversions. I'll reopen it until this is solved.

[sharkdp]

closed via #537