div, mod etc. give wrong results

[sostock]

IMO, the following results are wrong:

julia> div(5u"°C", 1u"°C") # Should be 1, or throw an error
5

julia> mod(5u"°C", 1u"°C") # Should be 4 °C, or throw an error
0 °C

As most other arithmetic operations on affine quantities error, these should probably too.

[sostock]

These do not seem right as well:

julia> div(1u"mV/V", 1) # should be zero
1 mV V^-1

julia> div(5u"m", 2) # should error
2 m

[cstjean]

I agree with you about division by affine quantities giving an error, but I'm not so sure in the general case. Someone using div on a unitful quantity almost certainly expects the currently implemented behavior:

julia> div(5u"m", 2) # should error
2 m

Of course, it's not invariant to the unit used, so it's not strictly a well-defined operation, but presumably the user will think about that. It would be frustrating to need a div (for whatever reason) and to have Unitful tell you that you can't do it.

[cstjean]

I mean, it's a judgement call of course, but IMO5u"°C" / 1u"°C" is a dangerously ambiguous operation that warrants an error, whereas div(5u"m", 2) only has one reasonable answer. div(1u"mV/V", 1) is unfortunate, but it would be ugly to special case.

[sostock]

I guess I don’t agree with that. IMO, div(5u"m", 2) has exactly zero reasonable answers, since div should always return an integer and never a dimensionful quantity.

I have reopened #354 which resolves the AffineQuantity issue, but I would like for this issue to stay open for now.

[giordano]

since div should always return an integer and never a dimensionful quantity.

Why? :thinking: I think such change would also be breaking, thus forcing us to go to v2

[sostock]

since div should always return an integer and never a dimensionful quantity.

Why? thinking I think such change would also be breaking, thus forcing us to go to v2

Because that is the definition of div. It is Euclidean division, i.e., the result of division, truncated to an integer. IMO, 2u"m" is not an integer.

I think div(5u"m", 2) is just as well-defined as div(5u"m", 2u"s"), which already errors. I can’t think of any useful application of div(5u"m", 2).

The fact that div(5u"m", 2) != div(500u"cm", 2) is another reason why this should not be allowed.

I would actually consider this a bugfix.

[ajkeller34]

The fact that div(5u"m", 2) != div(500u"cm", 2) is another reason why this should not be allowed.

There's a lot of past precedent for this sort of argument, e.g. in https://github.com/PainterQubits/Unitful.jl/issues/78. It makes me a little uneasy to have methods implemented that are not invariant under conversions of the input unit. I would be inclined to agree with you and roll back this behavior.

Of course, it's not invariant to the unit used, so it's not strictly a well-defined operation, but presumably the user will think about that. It would be frustrating to need a div (for whatever reason) and to have Unitful tell you that you can't do it.

It is risky to presume the user will think about it, that presumes the code is being written by someone who is thinking about physical units. I had intended for Unitful to work with generic functions that could have been written without Unitful in mind. I would much rather have div fail in that case and alert me to the incompatibility of that function with physical units. I agree it would be annoying when you just want to use Unitful like a calculator but it's just one extra call to ustrip to recover this functionality.

I would actually consider this a bugfix.

Seems a bit sneaky to consider it a bug fix and correspondingly only increment the patch version. After all, this behavior was explicitly introduced in v1.1. I think this is something to put on the todo list for v2.0.

[sostock]

One argument for the current behavior is that it’s consistent with the Dates stdlib, which allows Euclidean division of Periods by dimensionless numbers:

julia> using Dates

julia> Minute(10) ÷ 3
3 minutes

… which of course leads to the same problem (no invariance under unit conversion):

julia> Minute(2) == Second(120)
true

julia> div(Minute(2), 3) == div(Second(120), 3)
false

[cstjean]

Dates is very convenient, but it plays fast and loose with consistency, eg.

julia> DateTime(2020,2,29) + Day(1) + Month(1)
2020-03-30T00:00:00

julia> (DateTime(2020,2,29) + Day(1)) + Month(1)
2020-04-01T00:00:00

I wouldn't treat it as a standard to look up to...