within_abs() does not work with custom physical unit types

[alt-graph]

As Olaf observed, code like this does not work:

    Meters org_gap{ original_gap_*1_mm };
    if (gul14::within_abs(gap_mm, org_gap, 0.1_mm)) ...

... despite the fact that the Meters class has all the behavior of a common double. Maybe we can allow custom types. I wish we had concepts. :)

[Finii]

Has Meters some namespace? Where does it come from?

This tendency to omit namespaces is something I do look critical upon, and maybe I should flag such using statements more vigorously in reviews. It might look nicer the moment you write something, but revisiting after some years and the confusion is big.

[Finii]

Also gap_mm seems to be not a Meters type, or the variable name would be misleading.

I have the strong feeling the problem is due to an incomplete Meters implementation and not a GUL problem.

[alt-graph]

I guess Meters is hlc::units::Meters, but that does not really matter. The underlying problem is that within_abs() calls gul14::abs() which in turn falls back to std::abs() which is not defined for hlc::units::Meters (and may not be). It does have its own implementations of abs(), is_finite(), etc., which can be found via ADL. Maybe we can make use of that.

[Finii]

but that does not really matter

It does matter for me to be able to try it out :-D Will do that now.

[Finii]

My code, taken from the reported example above:

using namespace hlc::literals;

TEST_CASE("hlc units", "[num_util]") {
    auto original_gap_ = 10;
    auto gap_mm = 10.1;
    hlc::units::Meters org_gap{ original_gap_ * 1_mm };
    REQUIRE(gul14::within_abs(gap_mm, org_gap, 0.1_mm));
}

The first problem here is that gap_mm and org_gap have different types, which is not allowed, both must have the exact same type (NumT):

template<typename NumT>
bool within_abs(NumT a, NumT b, NumT tol) noexcept;

So the code is then

    REQUIRE(gul14::within_abs(hlc::units::Meters{ gap_mm * 1_mm }, org_gap, 0.1_mm));

gul14::abs() which in turn falls back to std::abs() which is not defined for hlc::units::Meters (and may not be). It does have its own implementations of abs(), is_finite(), etc., which can be found via ADL. Maybe we can make use of that.

The problem is not only abs() but also is_floating_point<>. And we are not allowed to specialize that either.

And using this Meters thing fails in more places, as we always expect types that have

std::is_floating_point_v<T> or std::is_integer_v<T> == true

For example

template<class NumT>
constexpr const NumT& clamp(const NumT& v, const NumT& lo, const NumT& hi) {
    return std::max(lo, std::min(v, hi)); // usually more optimized than actually using operator<()
}

The Meters types does not even have a std::numeric_limits<Meters> specialization. Without that it will be hard to support it in all kinds of generic functions GUL provides with the intention of non-user types.

Take for example Statistics:

template <typename DataT, typename = void, typename = std::enable_if_t<std::is_arithmetic<DataT>::value>>
struct MinMax {
    DataT min{ std::numeric_limits<DataT>::max() }; ///< Minimum value
    DataT max{ std::numeric_limits<DataT>::lowest() }; ///< Maximum value
};

template <typename DataT>
struct MinMax<DataT, std::enable_if_t<std::is_floating_point<DataT>::value>> {
    DataT min{ NAN };
    DataT max{ NAN };
};

The examples are endless and often we do checks for is_floating_point.

I guess it would be better to insist all the GUL functions work with 'normal' number types, and change the code above (which had a defect anyhow (wrong/mixed types) like

-    if (gul14::within_abs(gap_mm, org_gap, 0.1_mm)) ...
+    if (gul14::within_abs(gap_mm, hlc::units::counts<hlc::units::mm>(org_gap), 0.1)) ...

Hmm, just noticed that there is no way to get the mm out of a Meter back again, you are stuck with the base unit (unlike chrono for example where you can get the time in whichever base you want as double.

Not sure how to continue. Meters is just not numericy enough, I can not even imagine an implementation here, Meters breaks in so many places nut just within_abs().

I have no clue if we could work around some/all problems when Quantity provides a std::numeric_limits implementation. But maybe it is best to start there. And also add the missing get_the_value_out function.

:thinking:

[alt-graph]

In #61, I have tried to outline how we could solve this. There are still lots of parts missing, but it gets abs() and within_abs() working with Meters without supporting NaN and Inf. We can discuss if we want to pursue that direction.