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 runtime convertibility checks #110

Closed
chiphogg opened this issue Feb 18, 2023 · 3 comments · Fixed by #347
Closed

Provide runtime convertibility checks #110

chiphogg opened this issue Feb 18, 2023 · 3 comments · Fixed by #347
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 overflow safety surface is pretty useful, but it's also just a heuristic. It can be too restrictive in some cases, and even perhaps too permissive in a few.

In practice, unit conversions should never happen in hot loops. Thus, it would be nice if every unit conversion could be checked at runtime. These checks can be very efficient. We can generate one at compile time for every conversion. For overflow risk, we can simply compare the actual runtime value to the (compile-time constant) threshold. And for truncation error, we can perform the mod operation.

Really, the only thing stopping us is: what do we do when the check fails? Different error handling strategies are appropriate for different domains. There is no "one true error handling strategy".

Fortunately, we can separate out two steps: there's the error handling, and then there's the checking as to whether it should trigger in the first place. For the latter, we can provide functions which simply return bool. Then each project can make their own "checked conversion" function that handles errors in their preferred way.

@chiphogg chiphogg added the 📁 kind: enhancement New feature or request label Feb 18, 2023
@chiphogg chiphogg added ⬇️ affects: code (interfaces) Affects the way end users will interact with the library 💪 effort: medium labels Mar 26, 2023
@chiphogg chiphogg added this to the 0.4.0 milestone Jun 21, 2023
@chiphogg
Copy link
Contributor Author

chiphogg commented Dec 14, 2023

Checklist for features before we can call this "done".

  • Quantity conversions
    • "Basic" (i.e., same-rep)
    • Explicit rep
  • QuantityPoint conversions
    • "Basic" (i.e., same-rep)
    • Explicit rep

@chiphogg
Copy link
Contributor Author

(Note to future self.)

What work remains for the explicit-rep versions? It turns out that the explicit-rep unit conversions have three steps.

  1. Static cast the value to the common type of the reps.
  2. Perform the (same-rep) unit conversion.
  3. Static cast the result to the target rep.

The existing utilities cover step 2, and steps 1 and 3 are basically the same. Thus, the main thing we need is a tool to detect when a static cast is lossy. This could be pretty tricky in general, because we need to worry about:

  • signed and unsigned integral types
  • integral to floating point (remembering that there comes a point where floating point numbers cannot represent consecutive integers)
  • floating point to integral: this is OK if the value happens to be an exact integer!

It may seem that the first step, casting to the common type, is always lossless. This isn't true, even for the integers: the common type of a signed type and some other type can be an unsigned type, which is obviously lossy. So once we make our static cast checker, we will need to call it on both entry and exit.

@chiphogg chiphogg modified the milestones: 0.4.0, 0.3.6 Jul 24, 2024
chiphogg added a commit that referenced this issue Dec 8, 2024
The core enabling feature is the new "static cast checkers" library
target.  For each source and destination type, this target provides
separate checks for overflow and truncation when static casting.

We continue with our usual policy of treating floating point types as
"value preserving".  I did initially explore the possibility of treating
large integer inputs as "truncating" when converted to a floating point
type that can't represent them exactly.  However, I found this made the
library somewhat harder to reason about, for questionable benefit.
Additionally, I think it is fair to assume that users intentionally
entering the floating point domain have already accepted a kind of
"magnitude based reasoning", and trying to split hairs about preserving
exact integer values just felt too cute.

With these static cast checkers in hand, the explicit-rep runtime
conversion checkers become simple.  We check the static cast to the
common type, the unit conversion, and the final narrowing static cast to
the destination type.

To figure out how to write all these functions, I used some "fuzz-ish"
utilities, which generated random values of various integral and
floating point types, and performed various explicit-rep conversions.  I
checked that the round-trip conversion changed the value if-and-only-if
`is_conversion_lossy` was true.  After also taking intermediate sign
flips into account (to handle some signed/unsigned conversion edge
cases), I got to a point where integral-to-integral conversions always
gave the right result.  This gives me confidence in the overall
approach.  When floating point values came into the picture, I wasn't
able to design a fully satisfactory policy to avoid both false positives
and false negatives.  However, I did get to a point where the kinds of
errors I saw were ones I found acceptable, relating to "the usual
floating point error".  This was also what led me to embrace the policy
of simply treating floating point destination types as value-preserving,
consistent with the rest of the library.  This machinery is not ready
for prime time, but I may post it as an abandoned draft PR for
posterity.

I also updated the documentation, including making the floating point
policy explicit.

Fixes #110.
chiphogg added a commit that referenced this issue Dec 9, 2024
The core enabling feature is the new "static cast checkers" library
target.  For each source and destination type, this target provides
separate checks for overflow and truncation when static casting.

We continue with our usual policy of treating floating point types as
"value preserving".  I did initially explore the possibility of treating
large integer inputs as "truncating" when converted to a floating point
type that can't represent them exactly.  However, I found this made the
library somewhat harder to reason about, for questionable benefit.
Additionally, I think it is fair to assume that users intentionally
entering the floating point domain have already accepted a kind of
"magnitude based reasoning", and trying to split hairs about preserving
exact integer values just felt too cute.

With these static cast checkers in hand, the explicit-rep runtime
conversion checkers become simple.  We check the static cast to the
common type, the unit conversion, and the final narrowing static cast to
the destination type.

To figure out how to write all these functions, I used some "fuzz-ish"
utilities, which generated random values of various integral and
floating point types, and performed various explicit-rep conversions.  I
checked that the round-trip conversion changed the value if-and-only-if
`is_conversion_lossy` was true.  After also taking intermediate sign
flips into account (to handle some signed/unsigned conversion edge
cases), I got to a point where integral-to-integral conversions always
gave the right result.  This gives me confidence in the overall
approach.  When floating point values came into the picture, I wasn't
able to design a fully satisfactory policy to avoid both false positives
and false negatives.  However, I did get to a point where the kinds of
errors I saw were ones I found acceptable, relating to "the usual
floating point error".  This was also what led me to embrace the policy
of simply treating floating point destination types as value-preserving,
consistent with the rest of the library.  This machinery is not ready
for prime time, but I posted it for posterity in the abandoned PR #346.

I also updated the documentation, including making the floating point
policy explicit.

Fixes #110.
chiphogg added a commit that referenced this issue Dec 9, 2024
This fixes both a straight-up mistake, and an unclear example.

Follow-up for #110.
chiphogg added a commit that referenced this issue Dec 10, 2024
This fixes both a straight-up mistake, and an unclear example.

Follow-up for #110.
@chiphogg
Copy link
Contributor Author

I filed #352 as a follow-up for QuantityPoint, which I straight up forgot to do, but don't want to hold up the 0.4.0 release for.

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