Skip to content
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

Add support for zero #487

Closed
mpusz opened this issue Aug 31, 2023 · 15 comments
Closed

Add support for zero #487

mpusz opened this issue Aug 31, 2023 · 15 comments
Assignees
Labels
design Design-related discussion enhancement New feature or request iso The ISO C++ Committee related work

Comments

@mpusz
Copy link
Owner

mpusz commented Aug 31, 2023

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

@mpusz mpusz added enhancement New feature or request design Design-related discussion labels Aug 31, 2023
@mpusz
Copy link
Owner Author

mpusz commented Aug 31, 2023

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

std::transform_inclusive_scan(legs.cbegin(), legs.cend(), std::back_inserter(res), std::plus(), to_length,
distance::zero());

@mpusz
Copy link
Owner Author

mpusz commented Aug 31, 2023

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
Copy link
Owner Author

mpusz commented Aug 31, 2023

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

for (auto tt = 0 * ms; tt <= 50 * ms; ++tt) {

msl_altitude current_ = mean_sea_level + 0 * si::metre;

@mpusz
Copy link
Owner Author

mpusz commented Aug 31, 2023

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
Copy link
Owner Author

mpusz commented Aug 31, 2023

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 mpusz self-assigned this Aug 31, 2023
mpusz added a commit that referenced this issue Aug 31, 2023
@mpusz
Copy link
Owner Author

mpusz commented Aug 31, 2023

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
Copy link
Collaborator

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:

for (auto tt = 0 * ms; tt <= 50 * ms; ++tt) {

msl_altitude current_ = mean_sea_level + 0 * si::metre;

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
Copy link
Owner Author

mpusz commented Sep 1, 2023

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
Copy link
Owner Author

mpusz commented Sep 1, 2023

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
Copy link
Collaborator

chiphogg commented Sep 1, 2023

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 mpusz closed this as completed in cbcc6f4 Sep 13, 2023
@mpusz
Copy link
Owner Author

mpusz commented Dec 5, 2023

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?

@mpusz mpusz reopened this Dec 5, 2023
@JohelEGP
Copy link
Collaborator

JohelEGP commented Dec 5, 2023

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
Copy link
Owner Author

mpusz commented Dec 5, 2023

Isn't it portably implemented for comparison categories?

@JohelEGP
Copy link
Collaborator

JohelEGP commented Dec 5, 2023

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
Copy link
Owner Author

mpusz commented Jun 28, 2024

@mpusz mpusz closed this as not planned Won't fix, can't repro, duplicate, stale Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design-related discussion enhancement New feature or request iso The ISO C++ Committee related work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants