mpusz / mp-units

The quantities and units library for C++
https://mpusz.github.io/mp-units/
MIT License
1.07k stars 85 forks source link

Agnostic comparisons to zero? #524

Closed NAThompson closed 10 months ago

NAThompson commented 10 months ago

Consider the following code:

#include <vector>
template<typename T>
class polynomial {
public:
     polynomial(std::vector<T>&& coeffs) : c_{std::move(coeffs)} {
         while (c_.size() > 1 && c_.back()==T(0)) {
             c_.resize(c_.size() - 1);
         }
     }

     size_t degree() { return c_.size() - 1; }
private:
    std::vector<T> c_;
};

This code cannot operate with mp-units types due to the constraint documented here, and a workaround is documented here.

Nonetheless, I wonder if it is possible to make this code work for double and float whenever the mp-units library is not available, yet works in the expected way when mp-units types are provided. (Perhaps some __has_include work is possible, but it feels like a more elegant solution is available with more thought.)

I have tried to use the std functions via

#include <vector>
#include <compare>
template<typename T>
class polynomial {
public:
     polynomial(std::vector<T>&& coeffs) : c_{std::move(coeffs) {
         while (c_.size() > 1 && std::Is_sq(c_.back())) {
             c_.resize(c_.size() - 1);
         }
     }

     size_t degree() { return c_.size() - 1; }
private:
    std::vector<T> c_;
};

but no joy on clang-17.

chiphogg commented 10 months ago

You could define a special type, Zero, which is implicitly convertible to any numeric type, and also to any mp-units quantity. Here's a discussion on the concept from the Au docs: https://aurora-opensource.github.io/au/main/discussion/concepts/zero/

mp-units considered this approach instead of the current one. The reason it didn't end up getting accepted is that sometimes this Zero "acts like" a quantity, and sometimes it doesn't. In particular, passing it to a generic quantity interface will fail. The worry was that this would be too confusing to new users.

But that decision was only for what the library offers to all library users. There's nothing stopping you from making a type like this if it's right for your needs!

If you had such a type, you could write your example like this:

#include <vector>
template<typename T>
class polynomial {
public:
     polynomial(std::vector<T>&& coeffs) : c_{std::move(coeffs)} {
         while (c_.size() > 1 && c_.back()==Zero{}) {
             c_.resize(c_.size() - 1);
         }
     }

     size_t degree() { return c_.size() - 1; }
private:
    std::vector<T> c_;
};
NAThompson commented 10 months ago

@chiphogg : Makes sense. Ultimately I have a dream that all of Boost.Math will "just work" with any "reasonable" units library, simply by getting the templates sufficiently generic.

But it would appear that it's a bit harder than that. . .

mpusz commented 10 months ago

Did you try to use value intitialization?

 while (c_.size() > 1 && c_.back()==T{}) {
NAThompson commented 10 months ago

@mpusz : Works, thanks!

chiphogg commented 10 months ago

Value initialization will work, and in many ways it's an elegant solution. That said, I think the downside is that it reduces clarity of intent: it relies on the knowledge that T{} produces a value of zero. This is indeed true for all numeric types, and for mp-units quantity types, but T{} simply lacks the clarity of an alternative that says "0" or "zero" in some way.

As a counterpoint, Au is deliberately paranoid and standoffish about the value of T{} for its quantity types. That said, I have to admit that --- spoiler alert! --- we do produce a zero value from our default constructor, and despite our protestations that we reserve the right to change it, I find it hard to imagine producing any other value. Perhaps I'm tilting at windmills here. :slightly_smiling_face:

I guess as an alternative that preserves intent, you could write something like this?

constexpr auto ZERO = T{};
// ...
NAThompson commented 10 months ago

Maybe also add a static assert:

constexpr auto ZERO = T{};
static_assert(ZERO + ZERO==ZERO);

Zero is special in that it's the same in all units 0 m = 0cm = 0mm, and as such the rationale for disallowing T(0) is not compelling just for this case. I don't know how you'd treat this generically, but maybe you could have (say) T::ZERO which the standard could make work with double?

JohelEGP commented 10 months ago

If T{} equals 1 and T is a

set of two elements 0 and 1 with addition modulo 2

that static assertion passes.