units icon indicating copy to clipboard operation
units copied to clipboard

Effort to support integral underlying types

Open JohelEGP opened this issue 7 years ago • 8 comments

There is interest in having the library support integral underlying types. I believe that the duration class template of std::chrono sets a good example in doing so. Its interface is correct, efficient, easy to use right, and hard to misuse. I propose that we adopt the ways of std::chrono::duration to make this library more generally useful.

To properly support integral underlying types, the following changes need to be implemented. In summary, we start by providing support in the constructors and assignment operators of unit and their dependent convert free function. Then follows the specialization of std::common_type for units to serve as the base of safe conversions between units of the same dimension. Finally, we fix the operations on units to be consistent and behave like the underlying types their arguments represent.

  • The underlying type parameter of the unit constructors (1, 2) should be a template parameter of the constructors that only allows non-lossy conversions [ Note: duration uses treat_­as_­floating_­point_­v because it supports arithmetic-like types, for which the standard traits can't be specialized. To keep this effort simple, I think we should continue using the standard traits. -- end note ].
  • The converting constructor should prevent truncation and lossy conversions.
  • The convert function, used by the converting constructor,
    • needs to carry intermediate computations in the widest representation, and then
    • it should encourage type safety by having a type safe interface, or an alternative that has a type safe interface should be provided. A type safe interface would have a unit in the parameter and return types.
      • If an alternative is provided, convert should work with units instead of unit_conversions because units know the specific underlying types being handled, allowing convert to have a correctly specified parameter and return types.
  • Partially specialize std::common_type. This is pretty straight forward for linear units that don't need a translation, just like duration. Doing like the type safe convert for the other units should do the job, as that's what the converting constructor should continue using.
  • Provide support for the modulus operators to set an example for the other operators.
  • Fix the functions operating on units to work with the duration-like framework.
    • Equality operators.
    • Relational operators.
    • Arithmetic operators.
      • Linear.
      • Decibel.
    • cmath functions.

About the last main point, knowledge on the design of std::chrono is very important to correctly apply the framework to stuff std::chrono doesn't have, like the dimensional analysis operations and everything about the non-linear units. I've familiarized myself with it by watching videos about it by its main author, Howard Hinnant, as well as comments from him about std::chrono at StackOverflow, following the standardization process, using the library, reading its specification over and over, trying to generalize the duration interface, and probably in some other minor ways.

I'll try to list the aspects of the duration interface and implementation that will allow us to implement the changes listed above:

  • Implicit conversions that would result in loss of information are ill-formed. Just like our units of the same dimension which have an underlying type of double, floating-point durations are always convertible among each other. However, when it comes to integral durations, conversion from a floating-point duration to an integral duration can only be done explicitly with duration_cast, and implicit conversions between integral durations can only be done from durations, like seconds, to seconds or more precise durations that can exactly represent it, like milliseconds. Exactly means that you can't implicitly convert a half_a_second duration type to the more precise third_a_second duration type that have integral underlying type because the values of the former can't always be exactly represented in the later.
  • The construction from types convertible to the underlying type of a duration are also ill-formed if the conversion would result in a loss of information.
  • The compound assignment operators of duration take a duration of the same type or its underlying type as the rhs because everything will ultimately will happen on the underlying type of the lhs.
  • The other binary operators taking durations return the common type of the durations, and also convert its arguments to it before performing the operation on the underlying values and wrapping the result back. The common type of the durations represent the most precise duration to which both can be exactly converted (see this note). This formulation allows the operators to behave correctly and efficiently.
  • The rest of the binary operators taking a duration and a dimensionless type are also correct and efficient by operating on the same duration but with its underlying type being the common type of itself and the dimensionless type.

JohelEGP avatar May 11 '18 22:05 JohelEGP

I'll add that if we get this right, we should also include definitions for additional underlying type units.

nholthaus avatar May 11 '18 22:05 nholthaus

we should also include definitions for additional underlying type units.

What do you mean by that? Like how the standard 0s is an integral duration and .0s a floating-point duration?

JohelEGP avatar May 11 '18 23:05 JohelEGP

I think that's about it. The main comment should be complete now.

JohelEGP avatar May 12 '18 05:05 JohelEGP

What do you mean by that? Like how the standard 0s is an integral duration and .0s a floating-point duration?

I just mean defining types/alias like meter_t, only with an int underlying type. I'm planning to remove the _t's as part of the 3.0 refactor (although I'd gladly accept user feedback on that), so I'm not exactly sure what I'd want to call them. I think defining a single underlying type for the entire lib is too limiting, especially given your recent work.

I don't think the duration model will quite work for us because I still think the default/most readily accessible underlying type should be floating point.

nholthaus avatar May 15 '18 23:05 nholthaus

I just mean defining types/alias like meter_t, only with an int underlying type.

That'd be nice to have.

I'm planning to remove the _t's as part of the 3.0 refactor

That's great. I've read some things about that in previous issues and was planning to mention it to you because I didn't see anything about it for 3.0.0.

(although I'd gladly accept user feedback on that)

Then you'll want to open an issue for visibility.

I don't think the duration model will quite work for us because I still think the default/most readily accessible underlying type should be floating point.

The effort changes nothing in that respect.

JohelEGP avatar May 15 '18 23:05 JohelEGP

Are we ready to close this?

nholthaus avatar Jul 13 '18 22:07 nholthaus

Yeah. It seems like you've disabled closing issues from PRs.

JohelEGP avatar Jul 14 '18 00:07 JohelEGP

Right. I don't want them fully closed until they merge into master, but I like to tag them so I know that no more active quirk is required.

On Fri, Jul 13, 2018, 8:21 PM Johel Ernesto Guerrero Peña < [email protected]> wrote:

Yeah. It seems like you've disabled closing issues from PRs https://help.github.com/articles/closing-issues-using-keywords/.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nholthaus/units/issues/125#issuecomment-404984241, or mute the thread https://github.com/notifications/unsubscribe-auth/AJ2HH40J6DVnPCvIm4nXec64KPISrYDKks5uGTmlgaJpZM4T8JBD .

nholthaus avatar Jul 14 '18 12:07 nholthaus