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

Open
chiphogg opened this issue Jun 26, 2024 · 2 comments
Open

Provide more flexible alternative to integer_quotient() #253

chiphogg opened this issue Jun 26, 2024 · 2 comments
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.

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

No branches or pull requests

1 participant