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

Support mixed Constant-Quantity math functions #330

Merged
merged 2 commits into from
Nov 23, 2024

Conversation

chiphogg
Copy link
Contributor

Specifically, we add support for min, max, clamp, and %.

It turns out that the most economical way to do this is via hidden
friends. The downside is that this change is invasive to Quantity,
whereas we'd generally rather add functionality from the outside. But
the upsides are that we get to remove a fair bit of extra special casing
that we had done for Zero overloads, and even some disambiguating
overloads. Not only that, but we can now support some combinations that
we hadn't added before, simply because it would have been too much work!
Here's how it works.

The hidden friend approach covers us whenever somebody calls a function
with two exactly-identical types, or whenever one of the types is an
exact match, and the other can implicitly convert to it. This lets
us cover all "shapeshifter types" --- Zero, Constant --- at one
stroke. It even automatically covers new shapeshifter types we don't
know about
: anything implicitly convertible to Quantity will work!

The one downside is that using the unqualified forms of min, max,
and clamp, goes from "recommended" to "mandatory". We found some
instances of this in Aurora's code from testing this PR; they were
easily fixed by changing au::min(...) to min(...), etc.

For the min and max implementation, I went with the Walter Brown
approach where min prefers to return a, and max prefers b. This
is the most general and correct approach w.r.t. how it handles "ties",
although in our specific case this doesn't matter because we're not
returning a reference. Still, I'm glad to put one more example of the
Right Approach out in the wild, and I prefer it to a call to std::min
because it doesn't force us to take a direct dependency on <cmath>.

We have two "disambiguating" overloads remaining in math.hh, both
applying to QuantityPoint: one for min, one for max. I decided
not to add hidden friends there, because the cost of an invasive change,
plus the cost of moving these implementations far from the other
overloads in math.hh, outweighs the smaller benefits we would obtain
in this case.

Helps #90. At this point, the Constant implementation is feature
complete, and all we need to do is add concrete examples of Constant
to our library, updating the single-file package script and
documentation!

Specifically, we add support for `min`, `max`, `clamp`, and `%`.

It turns out that the most economical way to do this is via hidden
friends.  The downside is that this change is invasive to `Quantity`,
whereas we'd generally rather add functionality from the outside.  But
the upsides are that we get to remove a fair bit of extra special casing
that we had done for `Zero` overloads, and even some disambiguating
overloads.  Not only that, but we can now support some combinations that
we hadn't added before, simply because it would have been too much work!
Here's how it works.

The hidden friend approach covers us whenever somebody calls a function
with two exactly-identical types, _or_ whenever _one_ of the types is an
exact match, and the _other_ can _implicitly convert_ to it.  This lets
us cover all "shapeshifter types" --- `Zero`, `Constant` --- at one
stroke.  It even automatically covers _new shapeshifter types we don't
know about_: anything implicitly convertible to `Quantity` will work!

For the `min` and `max` implementation, I went with the Walter Brown
approach where `min` prefers to return `a`, and `max` prefers `b`.  This
is the most general and correct approach w.r.t. how it handles "ties",
although in our specific case this doesn't matter because we're not
returning a reference.  Still, I'm glad to put one more example of the
Right Approach out in the wild, and I prefer it to a call to `std::min`
because it doesn't force us to take a direct dependency on `<cmath>`.

We have two "disambiguating" overloads remaining in `math.hh`, both
applying to `QuantityPoint`: one for `min`, one for `max`.  I decided
not to add hidden friends there, because the cost of an invasive change,
plus the cost of moving these implementations far from the other
overloads in `math.hh`, outweighs the smaller benefits we would obtain
in this case.

Helps #90.  At this point, the `Constant` _implementation_ is feature
complete, and all we need to do is add concrete examples of `Constant`
to our library, updating the single-file package script and
documentation!
@chiphogg chiphogg added release notes: ✨ lib (enhancement) PR enhancing the library code release notes: 💥 lib (breaking change) PR making a breaking change labels Nov 23, 2024
@chiphogg chiphogg requested a review from geoffviola November 23, 2024 14:52
Tested with `au-docs-serve`
@chiphogg chiphogg merged commit 7791a9a into main Nov 23, 2024
13 checks passed
@chiphogg chiphogg deleted the chiphogg/constants-math-fns#90 branch November 23, 2024 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: 💥 lib (breaking change) PR making a breaking change release notes: ✨ lib (enhancement) PR enhancing the library code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants