search

nholthaus / units

a compile-time, header-only, dimensional analysis and unit conversion library built on c++14 with no dependencies.
http://nholthaus.github.io/units/
MIT License
964 stars 137 forks source link

raw() and value() is error prone #307

Open StefanoD opened 1 year ago

StefanoD commented 1 year ago

Please include the following information in your issue:

  1. Which version of units you are using 3.x (commit 70d5e3fdaea9afc1be7f84aa3a35031131330815)

We have an older version of the units lib (3.x, 2b2c063c87f934af79ae67006c35a9350d1b4a0a) where all units could be accessed with the value() method and you got the expected values. Later, this behavior changed for dimensionless units. This leads to unexpected behavior and actually our unit tests are failing now for the percent unit.

Our proposal is to introduce a unit proportion instead and fall back to the old behavior where the value of 5 percent actually returns 5 and not 0.05.

UNIT_ADD(concentration, proportion, prop, conversion_factor<std::ratio<100>, concentration::percent<>>)
andipeer commented 1 year ago

This change also prevented us from updating to the latest development version, as we were not sure if it will be kept like that or if we would need to again revert all the adaptions for the next stable release.

StefanoD commented 1 year ago

@nholthaus Is there any interest to restore the old behavior? If so, I could send you a PR and introduce proportion like suggested above.

JohelEGP commented 1 year ago

See also

What's really interesting is that the notion of "value" becomes ambiguous for every dimensionless unit, except the "unit one". There are two perfectly reasonable candidates for "the" value of such a quantity. A consequence which follows is that rather than try to guess which one the user meant, this kind of implicit conversion should be forbidden, as I explained here: nholthaus/units#301 (comment) -- https://github.com/mpusz/units/issues/412#issuecomment-1416904077.

ts826848 commented 1 year ago

It appears the old behavior might have been a bug, as the original intent was for to<>() and value() to produce the same result and the old behavior of value() was not doing that (and for what it's worth, to<>() and value() produced the same result in v2.x). raw() is presumably a way to allow access to the old behavior (where scaling information in the type is discarded) in case it's ever needed.