-
Notifications
You must be signed in to change notification settings - Fork 1
Allow custom numeric types in abs(), within_abs() #61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
[why] For a well-formed numerical type like "Meters", it was previously not possible to call gul14::abs() or gul14::within_abs(). This can now be done. [how] In the implementation of abs(), try to use std::abs(). If it is not available, fall back to calling a user-defined abs() via ADL. Signed-off-by: Lars Froehlich <lars.froehlich@desy.de>
|
Well, that STILL fails (it compiles but the resulting code is wrong), because template<typename NumT>
bool within_abs(NumT a, NumT b, NumT tol) noexcept {
tol = gul14::abs(tol); // Negative diff does not make sense
bool ret{};
if (a > b) {
if (std::is_floating_point<NumT>::value)
ret = a - tol <= b; // different formula needed because of inf/-inf and subnormal values
else
ret = a - b <= tol;
As But we can specialize But then, the problem occurs at a lot other GUL functions as well (I already pointed out the statistics functions), and it also happens at cmath functions (that is probably the reason [1] https://en.cppreference.com/w/cpp/types/is_arithmetic I do not really understand this. The
Rolling back my thoughts.
And
--- a/include/hlc/units/Quantity.h
+++ b/include/hlc/units/Quantity.h
@@ -124,7 +126,7 @@ public:
/// Explicitly cast this quantity to a floating-point value.
template <typename FloatT,
std::enable_if_t<std::is_floating_point<FloatT>::value, bool> = true>
- explicit constexpr operator FloatT() const noexcept
+ constexpr operator FloatT() const noexcept
{
return value_;
} |
|
I'm not sure if I understand this all correctly. To make this work, I also though, why not have implicit conversion and be done. But this will not fly, since you can not answer the question to what quantity you should convert without explicitly ask for the desired dimension. |
Exactly. We use physical unit types to avoid unit errors, so implicit conversions are a no-go. |
Yes. The very point of using a physical unit type is to avoid erroneous conversions, so having both |
That could be added. The point of this issue and MR is just to understand if it makes sense to continue working in this direction.
IMO we would also gain from implementing this for some subset, e.g. the
Absolutely, I just wanted to sketch the general idea first. |
Well, that (i.e. I guess we can rework this (i.e. GUL) after |
For a well-formed numerical type like "Meters", it was previously not possible to call gul14::abs() or gul14::within_abs(). This can be done with this PR.
How: In the implementation of
abs(), try to usestd::abs(). If it is not available, fall back to calling a user-definedabs()via ADL.This is a draft so far to see if an extension in this direction makes sense. We should discuss if a user-supplied implementation of
is_floating_point<T>is needed.