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
938 stars 134 forks source link

Scaling `100.0` does not recover all integral percentage values #275

Open chiphogg opened 3 years ago

chiphogg commented 3 years ago

This test fails for i in 29, 57, 58.

TEST(PercentT, RecoversInputValues) {
    for (int i = 0; i <= 100; ++i) {
        EXPECT_EQ(i, static_cast<int>(units::concentration::percent_t{i}.to<double>() * 100.));
    }
}

I think this ultimately stems from the inconsistent handling of percent_t vs. other units types, as elaborated in #81.


Please include the following information in your issue:

  1. Which version of units you are using: v2.3.1
  2. Which compiler exhibited the problem (including compiler version): gcc 7.5.0
nholthaus commented 1 year ago

added a test for this to 3.x

TEST(Consistency, recovers_input_values)
{
    for (int i = 0; i <= 100; ++i) {
        EXPECT_DOUBLE_EQ(i, units::concentration::percent<double>(i).value() * 100);
    }
}
chiphogg commented 1 year ago

Thanks for following up! But I don't think this test reproduces the problem. The goal of the original code was to recover the input value as an integer.

The static cast in the OP truncates. If by chance we end up slightly below, rather than slightly above, we're off by a whole 1: the consequences of small errors are asymmetrical.

I think this will always happen whenever anyone uses this strategy. It's a reasonable looking strategy at a glance, and might pass code review, but it'll just always be error prone.

The point of the original post (which was very much not obvious---sorry!) was that the real problem is that people are motivated to use this strategy, when what they want to do is to retrieve the original value, as if stored in a container.

It took me a long time to recognize this, but the notion of "value" is ambiguous specifically for dimensionless units other than 1. Consider 75%. By "value", in some contexts we might mean "the value of the quantity" (0.75), and in other contexts we might mean "the value stored" (75). This kind of dimensionless unit is the only kind where these notions are both different and not guarded by the type system.

In Aurora's library, I ended up deciding to forbid implicit conversions in both directions for this kind of unit. I don't know what solution is most appropriate for your library, but it might be nice to have some unambiguous way to say "retrieve the stored value in the given unit".