support common_type

[nholthaus]

[JohelEGP]

Are you considering supporting an interface that's more like std::chrono::duration for v3.0.0? As the library is now, the support for integral underlying types is poor. For example, it considers convertible units to be equally good convertible in both ways, and arbitrarily chooses to convert to the lhs type in binary functions. This potentially translates in a loss of precision, possibly turning a non-zero value to zero.

[johnmcfarlane]

As discussed previously, non symmetric binary arithmetic operators (except <<, >>) are typically ill-advised. From item 26 of C++ Coding Standards entitled Preserve natural semantics for overloaded operators:

Programmers expect operators to come in bundles. If the expression a @ b is well formed for some operator @ you define (possibly after conversions), ask: Can the caller also write b @ a without surprises?

I was certainly surprised to learn that these two expressions return different types for + and *.

[nholthaus]

@johnmcfarlane agree with you, but I think I'm missing a little context on what the two expressions you're referring to are?

Might be a stretch for 3.0.0, but we (or I) should get fixed point to work as an underlying type. I'm less than 100% motivated for int because I don't see units as a replacement for chrono, and int division renders most of the library units useless. However, fixed point makes a lot sense as a use case (to me).

Maybe in a seperate issue(s), but would be interested to hear if you've got any experiments or insight to share about that.

[JohelEGP]

He means that binary operations like 1_m + 1_cm has the same type as 1_m, but 1_cm + 1_m has the type of 1_cm.

I'm personally interested in having unit types based around a unit representing a quantity of pixels. Since most of the time it makes sense for it to be represented by an int, I'd like to offer my support to making the library integral representation friendly. I believe that std::chrono sets a good example in doing so.

I agree with you in the point about divisions, but what we do there is the same as we do for ints, because std::chrono handles itself so that conversions are performed as done for built-in types. For example, you might do i*.5 to get the value of an object i of type int halved, and you cast that back to int because that's what you're interested in storing. You can do the same with std::chrono. std::seconds{i}*.5 would result in a duration object representing seconds with an underlying type and a value equal to i*.5. But std::chrono goes further and forces you to be explicit with the cast back to an integral duration, because the floating-point to integral conversion is lossy.

[nholthaus]

So the assumption I was going off was that if conversions were implicit, that would be transparent to the user, since the final conversion is determined by the l-value type on the left-hand side anyway and so you wouldn't notice/care about the asymmetry. Intermediaries I didn't worry about because it doesn't affect the complexity of the conversion anyway.

I can see an additional duration-like interface being valuable, but a key design principle (maybe the only one) of this lib is allowing implicit conversions when it's safe to do so, even if it introduces some ambiguity. Whether or not you get strong dimensional analysis checking is based on whether you are explicit about the left-hand side type or not. It's permissive, but I think that's what makes it easy to use and document and is what the real value over boost::units is.

Based on the folks telling me that's not good, I'm guessing this becomes and issue with mixed underlying types, right? The current iteration of the library tacitly doesn't support mixing underlying types, because I didn't think there was a use case given how complex I think it will be to fix. Clearly I'm wrong (and actually have myself used int units for hardware register access and such), and I'd be aboard on being mixed-underlying-type friendly (or int friendly at a min) if we can find a path to getting there.

[JohelEGP]

That's good to hear!

I'm guessing this becomes and issue with mixed underlying types, right?

Yeah, it does. I'll try to come up with a plan when I'm done with the current light patches.

[JohelEGP]

I can see an additional duration-like interface being valuable, but a key design principle (maybe the only one) of this lib is allowing implicit conversions when it's safe to do so, even if it introduces some ambiguity. Whether or not you get strong dimensional analysis checking is based on whether you are explicit about the left-hand side type or not. It's permissive, but I think that's what makes it easy to use and document and is what the real value over boost::units is.

That sounds orthogonal to the effort of supporting mixed underlying types. The std::chrono::duration interface for floating-point underlying types is no less permissive than this library. As such, there's no need to burden the library with an additional interface.

[nholthaus]

can you show some pseudo-code of what you'd want the duration-like interface to look like? My mind is stuck on some type of units::quantity which I don't really like.

[JohelEGP]

What's the difference of that to units::unit? units::unit is good enough. It just needs the mixed-type support that std::common_type would give it in the return type, and a implementation of the functions that take that into account, in particular those that give preference to the lhs type. Beyond that, there's the strengthening of the interface by only allowing implicit conversions that don't change the value beyond what's already allowed. The dimensional analysis operations should also support mixed-types by having the underlying type of the result unit be a result of the std::common_type of the underlying types of the arguments.