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

feat: quantities can now be multiplied and divided by units #496

Closed mpusz closed 12 months ago

mpusz commented 1 year ago

Here are my thoughts after the implementation experience:

  1. Potentially more expensive when Rep is expensive to copy/move (quantity with a numeric value copied/moved many times before the final unit is obtained)
  2. Multiplication is commutative, so should m * 42 be a thing? This seems wrong to me.
  3. Should 42 / s construct a quantity? If so, how to define a quantity<1 / s> or define hertz with:
inline constexpr struct hertz : named_unit<"Hz", 1 / second, kind_of<isq::frequency>> {} hertz;

It seems that only multiplication after a number has a sense. Otherwise, we would have to heavily complicate the notation of inverses for units, dimensions, and quantity_specs (i.e. using std::integral_constant).

Please share feedback and if you are fine with merging that in.

JohelEGP commented 1 year ago

1 / second

How about also adding an inverse (commit f9ffacc713e00c6ea0207796f3eadd2ea4c5eb01) for units and/or references?

mpusz commented 1 year ago

How about also adding an inverse (commit https://github.com/mpusz/mp-units/commit/f9ffacc713e00c6ea0207796f3eadd2ea4c5eb01) for units and/or references?

I am not sure what would be the purpose of it? If it is to replace 1 / unit so it may construct quantity, I am not sure if that is a good idea. There are plenty of use cases where we want to specify an inverse unit and using a function could look worse. For example, quantity<inverse<s>> vs quantity<1 / s>.

JohelEGP commented 1 year ago

3. Should 42 / s construct a quantity? If so, how to define a quantity<1 / s> or define hertz with:

inline constexpr struct hertz : named_unit<"Hz", 1 / second, kind_of<isq::frequency>> {} hertz;

OK, I checked, and this is actually how hertz is currently specified in master. IIRC, 1 / second checks that the lhs is 1 (and returns a reference, not a quantity). mp_units::quantity is already a valid NTTP. If we decide we want 42 / s to construct a quantity, you could specialize named_unit for it, require that its number is 1, and give it the same meaning as in the master branch (i.e., inherit from that named_unit).

mpusz commented 1 year ago

you could specialize named_unit for it, require that its number is 1, and give it the same meaning as in the master branch (i.e., inherit from that named_unit).

I do not think that is a good idea ;-)

mpusz commented 1 year ago

One possible option could be to reuse magnitude for 1: quantity<mag<1> / s> or quantity<1_m / s>, but I am not sure if that is a good solution because it should also work for dimensions and quantity specs (i.e. frequency = mag<1> / time).

mpusz commented 1 year ago

Here is the list of the current use cases:

inline constexpr struct hertz : named_unit<"Hz", 1 / second, kind_of<isq::frequency>> {} hertz;
constexpr Unit auto my_unit = 1 / s;
quantity q = 42 * my_unit;
void foo(quantity<1 / s> q);
if constexpr(q.quantity_spec = 1 / isq::time) {}
if constexpr(q.dimension = 1 / isq::dim_time) {}
std::cout << q.in(1 / s) << "\n";
mpusz commented 1 year ago

With P2781 those would look as follows:

inline constexpr struct hertz : named_unit<"Hz", std::c_<1> / second, kind_of<isq::frequency>> {} hertz;
constexpr Unit auto my_unit = std::c_<1> / s;
quantity q = 42 * my_unit;
void foo(quantity<std::c_<1> / s> q);
if constexpr(q.quantity_spec = std::c_<1> / isq::time) {}
if constexpr(q.dimension = std::c_<1> / isq::dim_time) {}
std::cout << q.in(std::c_<1> / s) << "\n";
mpusz commented 1 year ago

With the inverse() proposed by Johel this will look like:

inline constexpr struct hertz : named_unit<"Hz", inverse(second), kind_of<isq::frequency>> {} hertz;
constexpr Unit auto my_unit = inverse(s);
quantity q = 42 * my_unit;
void foo(quantity<inverse(s)> q);
if constexpr(q.quantity_spec = inverse(isq::time)) {}
if constexpr(q.dimension = inverse(isq::dim_time)) {}
std::cout << q.in(inverse(s)) << "\n";
mpusz commented 1 year ago

We could also use identities:

Here is the list of the current use cases:

inline constexpr struct hertz : named_unit<"Hz", one / second, kind_of<isq::frequency>> {} hertz;
constexpr Unit auto my_unit = one / s;
quantity q = 42 * my_unit;
void foo(quantity<one / s> q);
if constexpr(q.quantity_spec = dimensionless / isq::time) {}
if constexpr(q.dimension = dimension_one / isq::dim_time) {}
std::cout << q.in(one / s) << "\n";
chiphogg commented 1 year ago

I like the inverse(...) and one approaches the best. Not a big fan of std::c_<1>, as it's clunky and long, with a teeny tiny payload. It's like a sample-sized whiskey bottle that comes in a big box full of packing peanuts.

inverse(...) reads nicely, IMO. And one is very close to what we have now, both in how it reads and in its length.