mpusz / mp-units

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

Add support for `zero` #487

Closed mpusz closed 1 week ago

mpusz commented 10 months ago

zero is really useful, as described here: https://aurora-opensource.github.io/au/main/discussion/concepts/zero.

mpusz commented 10 months ago

quantity::zero() can't be totally removed as it still has some use cases. For example:

https://github.com/mpusz/mp-units/blob/99bec8ecbb9dbebc9cfa6c7c1893cd70c9630485/example/glide_computer_lib/glide_computer_lib.cpp#L48-L49

mpusz commented 10 months ago

Should zero be limited only to addition, subtraction, and comparison? Or maybe the following should also be a thing:

static_assert((1 * m *= zero) == zero);
static_assert((1 * m /= zero) == zero);

static_assert((1 * m) * zero == zero);
static_assert(zero * (1 * m) == zero);
static_assert(zero / (1 * m) == zero);

I think that the above is controversial because it breaks the following logic:

1 * m + zero means adding a quantity of the same type with the value 0.

If we would like to extend that logic to 1 * m * zero, would it mean that we should end up with the quantity of area with the value 0? Or maybe it should mean (1 * m) * (0 * one) so as to treat zero as a dimensionless quantity with value one? If so, then 1 * m + zero should not work as we can't add dimensionless quantity to length.

mpusz commented 10 months ago

It seems that there are also some cases where zero should not be used even though it probably was meant to simplify such cases:

https://github.com/mpusz/mp-units/blob/99bec8ecbb9dbebc9cfa6c7c1893cd70c9630485/example/capacitor_time_curve.cpp#L42

https://github.com/mpusz/mp-units/blob/99bec8ecbb9dbebc9cfa6c7c1893cd70c9630485/example/unmanned_aerial_vehicle.cpp#L134

mpusz commented 10 months ago

In fact, after closely reviewing all our examples, the only place where zero could be used was here:

template<typename T>
struct MP_UNITS_STD_FMT::formatter<geographic::latitude<T>> : formatter<T> {
  template<typename FormatContext>
  auto format(geographic::latitude<T> lat, FormatContext& ctx)
  {
-    MP_UNITS_STD_FMT::format_to(ctx.out(), "{}", lat > geographic::latitude<T>::zero() ? 'N' : 'S');
-    return formatter<T>::format(lat > geographic::latitude<T>::zero() ? lat.numerical_value() : -lat.numerical_value(), ctx);
+    MP_UNITS_STD_FMT::format_to(ctx.out(), "{}", lat > mp_units::zero ? 'N' : 'S');
+    return formatter<T>::format(lat > mp_units::zero ? lat.numerical_value() : -lat.numerical_value(), ctx);
  }
};

template<typename T>
struct MP_UNITS_STD_FMT::formatter<geographic::longitude<T>> : formatter<T> {
  template<typename FormatContext>
  auto format(geographic::longitude<T> lon, FormatContext& ctx)
  {
-    MP_UNITS_STD_FMT::format_to(ctx.out(), "{}", lon > geographic::longitude<T>::zero() ? 'E' : 'W');
-    return formatter<T>::format(lon > geographic::longitude<T>::zero() ? lon.numerical_value() : -lon.numerical_value(), ctx);
+    MP_UNITS_STD_FMT::format_to(ctx.out(), "{}", lon > mp_units::zero ? 'E' : 'W');
+    return formatter<T>::format(lon > mp_units::zero ? lon.numerical_value() : -lon.numerical_value(), ctx);
  }
};

I am not sure if this is enough to justify complicating design with new features and a bunch of overloads 🥴

mpusz commented 10 months ago

Maybe just our short examples are not representing the production code base well enough? In bigger projects people will have quantity types stored in some structures and not deduced by auto so initialization with zero might simplify things.

On the other hand, it is probably much better to benefit from the zero-initialization...

mpusz commented 10 months ago

As I am puzzled about what to do with this feature, I pushed it to PR #488 so you can assess the benefits and changes required to implement it.

Please let me know what you think.

chiphogg commented 10 months ago

Should zero be limited only to addition, subtraction, and comparison? Or maybe the following should also be a thing:

static_assert((1 * m *= zero) == zero);
static_assert((1 * m /= zero) == zero);

static_assert((1 * m) * zero == zero);
static_assert(zero * (1 * m) == zero);
static_assert(zero / (1 * m) == zero);

I think that the above is controversial because it breaks the following logic:

1 * m + zero means adding a quantity of the same type with the value 0.

If we would like to extend that logic to 1 * m * zero, would it mean that we should end up with the quantity of area with the value 0? Or maybe it should mean (1 * m) * (0 * one) so as to treat zero as a dimensionless quantity with value one? If so, then 1 * m + zero should not work as we can't add dimensionless quantity to length.

I hadn't thought of multiplicative use cases before!

The way to think of zero is that it is "0 in whatever type it needs to be". This implies that there is such a type we can unambiguously identify. If that's not the case, it's probably not a good use case for zero. And in any case, we can satisfy the multiplicative use cases by simply using 0.

It seems that there are also some cases where zero should not be used even though it probably was meant to simplify such cases:

https://github.com/mpusz/mp-units/blob/99bec8ecbb9dbebc9cfa6c7c1893cd70c9630485/example/capacitor_time_curve.cpp#L42

https://github.com/mpusz/mp-units/blob/99bec8ecbb9dbebc9cfa6c7c1893cd70c9630485/example/unmanned_aerial_vehicle.cpp#L134

Yeah, zero really only helps when the type that it "shapeshifts" into is already clearly defined. So I don't think the first case is really even something that zero is intended to help with. (zero is good when the unit is really complicated and hard to spell. But in this case, even if that were true for the first argument 0 * ms, then it'd still be true for the second argument 50 * ms, and zero can't help with that anyway!)

Also --- and this may be vague and hard to express --- but it feels like 0 * ms here isn't a "fundamental zero" use case, but only a quantity whose value happens to be 0.

As for the second example, it wasn't clear to me why zero wouldn't be able to replace 0 * si::metre.

In fact, after closely reviewing all our examples, the only place where zero could be used was here:

template<typename T>
struct MP_UNITS_STD_FMT::formatter<geographic::latitude<T>> : formatter<T> {
  template<typename FormatContext>
  auto format(geographic::latitude<T> lat, FormatContext& ctx)
  {
-    MP_UNITS_STD_FMT::format_to(ctx.out(), "{}", lat > geographic::latitude<T>::zero() ? 'N' : 'S');
-    return formatter<T>::format(lat > geographic::latitude<T>::zero() ? lat.numerical_value() : -lat.numerical_value(), ctx);
+    MP_UNITS_STD_FMT::format_to(ctx.out(), "{}", lat > mp_units::zero ? 'N' : 'S');
+    return formatter<T>::format(lat > mp_units::zero ? lat.numerical_value() : -lat.numerical_value(), ctx);
  }
};

template<typename T>
struct MP_UNITS_STD_FMT::formatter<geographic::longitude<T>> : formatter<T> {
  template<typename FormatContext>
  auto format(geographic::longitude<T> lon, FormatContext& ctx)
  {
-    MP_UNITS_STD_FMT::format_to(ctx.out(), "{}", lon > geographic::longitude<T>::zero() ? 'E' : 'W');
-    return formatter<T>::format(lon > geographic::longitude<T>::zero() ? lon.numerical_value() : -lon.numerical_value(), ctx);
+    MP_UNITS_STD_FMT::format_to(ctx.out(), "{}", lon > mp_units::zero ? 'E' : 'W');
+    return formatter<T>::format(lon > mp_units::zero ? lon.numerical_value() : -lon.numerical_value(), ctx);
  }
};

I am not sure if this is enough to justify complicating design with new features and a bunch of overloads 🥴

Maybe just our short examples are not representing the production code base well enough? In bigger projects people will have quantity types stored in some structures and not deduced by auto so initialization with zero might simplify things.

On the other hand, it is probably much better to benefit from the zero-initialization...

I grepped our codebase for \bZERO\b in C++ files and found over a thousand results. There were a few false positives, but most of them were true positives: either initialization or comparison use cases. This meshes well with my personal experience that it's really handy to have around, and has made the library easier to use effectively.

mpusz commented 10 months ago

As for the second example, it wasn't clear to me why zero wouldn't be able to replace 0 * si::metre.

If we replace 0 * si::metre with zero, then mean_sea_level + zero does not know anything about the unit it should provide for the resulting quantity_point as mean_sea_level knows only about isq::altitude but not about the unit as well.

mpusz commented 10 months ago

I grepped our codebase for \bZERO\b in C++ files and found over a thousand results. There were a few false positives, but most of them were true positives: either initialization or comparison use cases. This meshes well with my personal experience that it's really handy to have around, and has made the library easier to use effectively.

Thanks, that convinces me to merge this feature :-)

chiphogg commented 10 months ago

As for the second example, it wasn't clear to me why zero wouldn't be able to replace 0 * si::metre.

If we replace 0 * si::metre with zero, then mean_sea_level + zero does not know anything about the unit it should provide for the resulting quantity_point as mean_sea_level knows only about isq::altitude but not about the unit as well.

Oh, good point --- this is just my lack of experience with mp-units quantity point showing. Makes sense!

mpusz commented 7 months ago

I have just learned the following: https://godbolt.org/z/689Wa188h. We can treat literal 0 as a special value not being an int. Can we somehow benefit from that in the features discussed here?

JohelEGP commented 7 months ago

That's the behavior of the built-in <=>: https://eel.is/c++draft/cmp.categories.pre#3. You can see how library implementations do it. This has been discussed a few times on the Cpplang slack. If you search for in:#general cmp.categories.pre there, you'll find that we came to the conclusion that, IIRC, we can't portably implement this behavior.

mpusz commented 7 months ago

Isn't it portably implemented for comparison categories?

JohelEGP commented 7 months ago

I suppose you could detect the C++ standard library being used, and do it their way. With a branch for when you don't recognize it, and use a more fragile implementation.

Maybe the fragile implementation will always work for our use case. And unless the C++ standard library implementation relies on compiler support, it too is bound to have some kind of observable difference in some way where something else that literal 0 has a surprising meaning.

mpusz commented 1 week ago

https://cplusplus.github.io/LWG/issue4051