update rounding code

[BeastyBlacksmith]

This updates the rounding code to the post 1.0 era.
To me rounding to significant digits is also well defined for Quantities.
I need to adjust the tests.

[BeastyBlacksmith]

probably you want to keep the old methods for backwards compability?

[codecov-io]

Codecov Report

Merging #246 into master will increase coverage by 0.29%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #246      +/-   ##
==========================================
+ Coverage   77.72%   78.02%   +0.29%     
==========================================
  Files          15       15              
  Lines        1037     1051      +14     
==========================================
+ Hits          806      820      +14     
  Misses        231      231

Impacted Files	Coverage Δ
src/quantities.jl	91.25% <100%> (+0.72%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0dbc96d...a8a400c. Read the comment docs.

[ajkeller34]

Thanks, I agree a 1.0 update is useful, but I don't think rounding to significant digits is well defined for dimensionful quantities. It seems like things work if the units differ by powers of ten, but otherwise, the operation is not invariant under a change of units. Using your branch:

julia> l = 1u"cm"
1 cm

julia> floor(l; sigdigits=1) == floor(uconvert(u"km", l); sigdigits=1)
true

julia> floor(l; sigdigits=1) == floor(uconvert(u"inch", l); sigdigits=1)
false

For some context, this kind of operation was first prohibited after #78. There was some discussion on making safe methods of round for dimensionful quantities in #207.

[c42f]

I think the two-argument form of #207 is reasonable, but I do wonder whether the inch-vs-km example is related to units as such. That is, given

julia> l = 1.23
julia> factor = 1.7

it's not surprising that the following versions using Base.floor are not equal:

julia> floor(l*factor, sigdigits=2)/factor
1.1764705882352942
julia> floor(l, sigdigits=2)
1.2

To the extent that these kind of functions are used for display of data to humans, I find this completely reasonable behavior. On the other hand, surprises like #78 are no good.

[BeastyBlacksmith]

I think the issue with

floor(l; sigdigits=1) == floor(uconvert(u"inch", l); sigdigits=1) # false

is rather promotion of rounded quantities than rounding to significant digits itself.
Personally I wouldn't suspect this equality to hold but to prevent issues it would be good to have a RoundedQuantity type and then define

promote_rule(x::RoundedQuantity,y::Any) = error("Rounded quantities cannot be promoted due to loss of precision")

or something like that.

[ajkeller34]

@c42f I get what you're saying and I agree there are usability compromises that must be made eventually. But in your example, you are passing round different values. On the other hand, from a physical perspective, 1inch and 2.54cm are different representations of the same value. It's the same length, and I think you're taking the view that round should ignore that these are lengths and just consider them as real numbers, which might be practical but is not in the spirit of other design choices made in the package (for instance we don't have Quantity{Float64} <: Real). I think a closer analogy would be that round(1.23f0) == round(1.23); representations of the same value in different types ought to return the same result.

[c42f]

On the other hand, from a physical perspective, 1inch and 2.54cm are different representations of the same value. It's the same length, and I think you're taking the view that round should ignore that these are lengths and just consider them as real numbers

To be clear, I'm not so much arguing for the changes in this PR as trying to understand the shape or the problem better by thinking out loud ;-) The point I was trying to make is that rounding is inherently dependent on the numerical representation because it's defined by finding the closest representable number in the base 10 positional numeral system with a given number of significant digits. Choose a different radix or numeral system and the set of representable numbers isn't even the same. The choice of representation in inch vs km gives a different lattice of numbers for the rounding operation, in some ways not dissimiiar from changing the radix.

In any case thanks for explaining the larger design considerations surrounding this. I think your analogy to round(1.23f0) == round(1.23) is quite accurate.

[BeastyBlacksmith]

so I did rebase and added the two and three argument forms as discussed in #249.
So give it a try and see if it works as expected.