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

I miss value_cast<ToQ> and potentially value_cast<ToQP> #568

Open
burnpanck opened this issue May 10, 2024 · 11 comments · May be fixed by #571
Open

I miss value_cast<ToQ> and potentially value_cast<ToQP> #568

burnpanck opened this issue May 10, 2024 · 11 comments · May be fixed by #571

Comments

@burnpanck
Copy link
Contributor

burnpanck commented May 10, 2024

I see that the library now offers casts that are explicit in what they do - I like! I currently see the following overloads:

  • value_cast<ToU>
  • value_cast<ToRep>
  • value_cast<ToU,ToRep>

These exist for both quantities and quantity points. For quantities, I miss the overload value_cast<ToQ>, which is equivalent to value_cast<ToQ::unit,To:Q::rep>, but shorter.

Furthermore, I argue that qp.point_for(ToPO) is in fact also a cast of the same style: changing the representation without changing the meaning, but potentially raising truncation issues. Thus, I propose to add a value_cast<ToPO> to the overload set. If that is acceptable, then the corresponding value_cast<ToQP>(qp) (approximately equivalent to either value_cast<ToPQ::unit,ToPQ::rep>(qp).point_for(ToPQ::point_origin) or value_cast<ToPQ::unit,ToPQ::rep>(qp.point_for(ToPQ::point_origin))) would be even more valuable, as being more concise and potentially useful. That said, in those cases it is slightly more difficult to guess how severe the overall truncation is, and the two almost equivalent two-step casts may have different truncation behaviours, with neither being always better. That said, for floating-point types, the value_cast<ToPQ::unit,ToPQ::rep>-part is usually considered value-preserving, so that there is no order ambiguity left. Even for integral types, it may be beneficial to offer a one-step cast, which may intelligently choose how to perform the cast best.

@mpusz
Copy link
Owner

mpusz commented May 10, 2024

Quantity type can be changed with a quantity_cast<ToQS>. As those are separate casts with different side effects, we do not have the cast taking the entire quantity type as a target as this would potentially require casting the value (rep and unit) and quantity type (quantity spec).

@mpusz
Copy link
Owner

mpusz commented May 10, 2024

Furthermore, I argue that qp.point_for(ToPO) is in fact also a cast of the same style: changing the representation without changing the meaning, but potentially raising truncation issues. Thus, I propose to add a value_cast to the overload set.

I am not so sure if a translation between vector spaces is a cast operation?

@burnpanck
Copy link
Contributor Author

burnpanck commented May 10, 2024

quantity_cast<ToQS> does something different: It allows changing the quantity type. What I was suggesting precisely was that value_cast<ToQ>(q) shall be equivalent to value_cast<ToQ::unit,ToQ::rep>(q) precisely iff the quantity type of q and ToQ is the same (or implicitly convertible), or else shall fail to compile. Thus, if I do value_cast<ToQ>, I'm guaranteed not to change the interpretation, just the representation.

@burnpanck
Copy link
Contributor Author

I am not so sure if a translation between vector spaces is a cast operation?

Here, my line of though is again in "representation" vs "semantic meaning". For me qp.point_for(ToQP) is not a change of vector space, but a change of "coordinate system". If I want to describe to you where I live, I could chose any popular city's city-hall that you know, and tell you how many meters east I live. I could instead tell you a number of miles with respect to the same city. Or I could choose a different city. Every time, I need to adjust the number that I give you, but we're always talking about the same point, my home. Thus, in addition to the "number", I need two things of metadata to tell you what this number represents; the reference point and the unit of measurements. Furthermore, if I write down the number on paper, I could either choose scientific notation (1.23 x 10^4) or an integer (12300). So the number representation is just the third choice I have in the representation.

Of course, this only applies as long as the reference points are all embedded in the same vector space; exactly the same requirement as for point_for(ToPO). I'm not saying that point_for(ToPO) is not valuable, or even that value_cast<ToPO> would be particularly intuitive. But I do think that value_cast<Q> and value_cast<QP> are valuable (I need them all over our code): I usually start from a narrow integer representation using a custom unit and reference point as read out by a piece of hardware (say, an uint16 measuring in units of 2^-7 degree Celsius, with zero corresponding to -45 degree Celsius). I keep that representation around for as long as possible, but eventually, it may go into a feedback loop, which expects a completely different representation (floating point, units of 1 degree Celsius, reference point + 30 degree Celsius). At this point, I don't care anymore that I started off with a specific sensor's representation, I just want that temperature point here. I'm aware that my sensor has a different measurement resolution and value range than the target quantity point, but I'm fine with that. So I ask my chosen units library to please do the conversion for me, I still want the same temperature point, up to small rounding issues.

@mpusz
Copy link
Owner

mpusz commented May 10, 2024

Sure, I understand. Do you have good motivating examples for such a use case, as I will need to defend this in the ISO C++ Committee? Said otherwise, do you often change both a unit and a representation type?

@burnpanck
Copy link
Contributor Author

burnpanck commented May 10, 2024

We cross-posted, but the second paragraph (minus the point-origin discussion) also is the motivating example for the value_cast<ToQ> use-case: Starting-off from a representation close to the sensor hardware, which is typically not exactly represented in SI units (but defined with respect to an SI unit), going to a unit that is an actual SI unit; widening the representation in the process (often just float - but only because my microcontroller does in fact have a floating-point coprocessor).

E.g. a lidar sensor could output as quantity<isq::distance[pow<2,-25>*si::second*si::speed_of_light],uint16> (counting time using a 32 MHz clock), but eventually, I may want to have quantity<si::milli<si::metre>,int32>. The source type often comes in as a generic function argument, and the output type may be specified as part of a member variable, so I may not actually know the exact choice of unit nor representation type in that part of the code, and instead rely on the people choosing the sensor and architecting the API to choose sensible units.

@mpusz
Copy link
Owner

mpusz commented May 10, 2024

OK, so we can consider adding those overloads. I would prefer to be conservative first and require quantity specs to be exactly the same. Unless you have a good motivation example for the implicitly convertible ones?

Unfortunately, I am unable to implement those things now as we have an ISO C++ Committee mailing deadline 12 days from now, and I scope on writing the next revisions of the papers. It sucks, but most of my time lately is spent on writing those papers instead of extending the library. And the backlog of things to design and implement grows all the time...

Can you provide a PR?

@burnpanck
Copy link
Contributor Author

I'll try. I do have a deadline for this refactor myself (a customer needs a firmware update in just 7 days from now).

@mpusz
Copy link
Owner

mpusz commented May 10, 2024

Thanks a lot! And good luck. A week for a refactor is not that much at all.

@burnpanck
Copy link
Contributor Author

How do I test "equality" of quantity specs?

@mpusz
Copy link
Owner

mpusz commented May 10, 2024

Just use op==.

@burnpanck burnpanck linked a pull request May 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants