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

Provide more flexible alternative to integer_quotient() #253

Closed
chiphogg opened this issue Jun 26, 2024 · 3 comments · Fixed by #315
Closed

Provide more flexible alternative to integer_quotient() #253

chiphogg opened this issue Jun 26, 2024 · 3 comments · Fixed by #315
Labels
⬇️ affects: code (interfaces) Affects the way end users will interact with the library 📁 kind: enhancement New feature or request 💪 effort: medium
Milestone

Comments

@chiphogg
Copy link
Contributor

The point of integer_quotient() is to make it extremely obvious that we are about to do a possibly-truncating integer division. We have seen that this is very error prone when the denominator has units, and when those units are not quantity-equivalent to the numerator's units. So it's good that we've put up this guard rail.

However, integer_quotient() is very inflexible. For one thing, it's horrible for generic programming: there's no real way to write a division operation that sometimes uses integers (and you're OK with the truncation), but sometimes uses floating point. For another, it looks completely different to the division operation.

I think we can get all of the safety benefits of integer_quotient() by introducing a wrapper type for the denominator. I don't know what the name should be yet, but it would be something like this:

constexpr auto distance = meters(60);
constexpr auto speed = (miles / hour)(65);

// Old way:
{
    // Using `integer_quotient` says "yes, I really want this to truncate (even to 0)".
    constexpr auto time = integer_quotient(distance, speed);
}

// New way?
{
    constexpr auto time = distance / unblock_integer_division(speed);
}
@chiphogg chiphogg added 📁 kind: enhancement New feature or request 💪 effort: medium ⬇️ affects: code (interfaces) Affects the way end users will interact with the library labels Jun 26, 2024
@chiphogg chiphogg added this to the 0.3.6 milestone Jul 24, 2024
@chiphogg
Copy link
Contributor Author

I got a local implementation done a couple weeks ago, using the name unblock_int_div. But maybe it would be better to call it something else, so we could use the same construct to handle %. Consider the following:

constexpr auto t1 = hours(5);
constexpr auto t2 = minutes(120);
constexpr auto from_quotient_and_remainder = (t1 / t2) * t2 + (t1 % t2);

Currently, the (t1 / t2) wouldn't compile, of course. But if it did, we'd get (hours / minute)(5 / 120), that is, (hours / minute)(0). When multiplying by t2, we get hours(0).

t1 % t2, on the other hand, first automatically converts to the common unit (here, minutes) --- as it should, because this is a common-unit operation, and it is unambiguously well defined as a pure quantity operation. This is equivalent to subtracting t2 from t1 repeatedly until you can't anymore, and returning what's left. It gives minutes(60).

Put it all together, and we get minutes(60) overall. But this is just the quotient-remainder theorem, and we should recover a quantity equivalent to the original input of hours(5)!

When we previously discussed this example, I thought that the best way to handle each of division and modulus individually was crystal clear, but when you put them together, they don't satisfy the quotient-remainder theorem. My solution was to propose a separate quotient_and_remainder function that returns both values.

I still think that function should exist, but I've realized now that there are two different modulus operations, when it comes to units. There's the "quantity-like" modulus operation, which is very clearly a common-unit operation, and is equivalent to what's left over from repeated subtraction. And then there's the "quotient-and-remainder-like" modulus operation, which holds whatever we need in order to make (integer) division invertible. And since division is an arbitrary-unit operation, then so is this modulus operation.

Let's look at some examples to show how this new modulus operation would work. I'm assuming we'll call the utility something like use_raw_num, such that t1 % use_raw_num(t2) would be the new modulus operation, the use_raw_num in t1 / use_raw_num(t2) would unblock integer division in case t1 / t2 was forbidden (and would be a no-op otherwise). I've called it raw for short in the tables below, to avoid wrapping. "Q/R" means "result of quotient-remainder theorem". Here we go:

t1 t2 (t1 / t2) * t2 t1 % t2 t1 % raw(t2) Q/R (%) Q/R (% raw)
5 * h 120 * m 0 * h 60 * min 5 * h 60 * min ✔️ 5 * h
5 * min 1 * h 5 * min 5 * min 0 * min 10 * min ✔️ 5 * min
5 * min 2 * h 4 * min 5 * min 1 * min 9 * min ✔️ 5 * min
5 * min 2 * g 4 * min 1 * min ✔️ 5 * min

We see that t1 / use_raw_num(t2) * t2 + t1 % use_raw_num(t2) always gives t1, whereas (at least for these examples) t1 / t2 * t2 + t1 % t2 never does. In fact, use_raw_num even handles some cases that could never even compile with the quantity-like modulus!

In fact, we get more than I had expected. I was assuming we would always recover the correct quantity, although the units might be different. (This was based on intuition from the quantity-like modulus, where hours(5) % minutes(120) would give minutes(60).) In fact, we get the right answer expressed in the original unit, every time! That's pretty interesting.

So I think I'll try to make use_raw_num work for both / and %. For the latter, it will use the underlying raw numbers, and return the units of the left-hand argument.

@chiphogg
Copy link
Contributor Author

Putting this in the 0.4.0 milestone so that we can begin the deprecation pipeline for integer_quotient() ASAP.

@chiphogg
Copy link
Contributor Author

I have thought through the naming issues more thoroughly now. I don't think we need to block this issue on finding a name that properly conveys both integer division and "q-r-like modulus". Yes, these are related operations. But they are not the same thing, and trying to give them the same name would likely just cause unnecessary confusion.

unblock_int_div is a great name for this issue. "Unblock" says that it was blocked before. "int div" highlights that integer division (with its truncating property) can now be used here. If we tried to make the result of this operation also mean "use the arbitrary-unit version of mod, which makes the Q/R theorem work", then we would have to use a name that would be less clear than unblock_int_div for this promary use case.

We'll need some other name if we decide to provide this second form of modulus (we haven't seen any actual use cases yet). Maybe use_raw_mod, maybe not. But taking that as a placeholder for now, the quantity/remainder theorem would be expressed:

t1 == (t1 / unblock_int_div(t2)) * t2 + t1 % use_raw_mod(t2);

chiphogg added a commit that referenced this issue Oct 29, 2024
Instead of `integer_quotient(a, b)`, we now always write
`a / unblock_int_div(b)`.  This gives us everything we used to have with
`integer_quotient`, but with two big advantages:

1. The form for the units code becomes more similar to the non-units
   code that it replaces (i.e., we're using the `/` symbol).

2. We can now support _templated code_ that works for both integral and
   non-integral types: `a / unblock_int_div(b)` works equally well for,
   say, `b` with floating point rep.

There's no longer any use case for `integer_quotient`.  We therefore
immediately deprecate it.  We will keep it deprecated for the entirety
of the 0.4.0 cycle, and remove it for 0.5.0.

Fixes #253.
chiphogg added a commit that referenced this issue Oct 30, 2024
Instead of `integer_quotient(a, b)`, we now always write
`a / unblock_int_div(b)`.  This gives us everything we used to have with
`integer_quotient`, but with two big advantages:

1. The form for the units code becomes more similar to the non-units
   code that it replaces (i.e., we're using the `/` symbol).

2. We can now support _templated code_ that works for both integral and
   non-integral types: `a / unblock_int_div(b)` works equally well for,
   say, `b` with floating point rep.

There's no longer any use case for `integer_quotient`.  We therefore
immediately deprecate it.  We will keep it deprecated for the entirety
of the 0.4.0 cycle, and remove it for 0.5.0.

Fixes #253.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⬇️ affects: code (interfaces) Affects the way end users will interact with the library 📁 kind: enhancement New feature or request 💪 effort: medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant