implement value_cast<ToQ> and value_cast<ToQP>

[burnpanck]

Fixes #568

[mpusz]

BTW, instead of [WIP] in the title you can set PRs as "draft" until they are ready. I just did it for this PR.

[burnpanck]

Given that it is somewhat nontrivial to implement a correct general value_cast<ToQP>(qp) which allows adjustments in the point_origin, I believe there is value in having this in the library. The challenge is that neither FromQP nor ToQP may have enough range to reach each other's point_origin. I believe the provided implementation manages to achieve a result within std::max(FromQP::epsilon,ToQP::epsilon) of the exact conversion result, assuming qp is within the range of ToQP. Otherwise, the conversion is unspecified (any may well be undefined behaviour, e.g. if the representation is a signed integer).

[burnpanck]

Oh, shall I try to sqash?

[mpusz]

I think you are assuming here an integral representation type, which often will not be the case as double is the default in the library. In cases of a floating point representation type, we should calculate the entire conversion factor in one step and then do only one multiplication operation on the value to get the result. Otherwise, we will not generate assembly equivalent to operations on double.

[mpusz]

If my previous comment was unclear, I will just note that the relative_point_origin stores an offset, which has its own unit and representation as well. We could factor in this conversion here as well (instead of calling point_for() as a separate step).

However, maybe it would be too complex to do it here anyway and the measurable performance gains would not be that big.

[burnpanck]

Indeed, those considerations about order of the conversion operations are most relevant for integral types, where both underflow and overflow are likely. It doesn't hurt for double either. However, I'm not sure what you mean by "calculate the entire conversion factor in one step". A change of point-origin requires an addition. A change in unit requires a multiplication. This implementation here does at most one multiplication and at most one addition in all cases. Here is how:

• The initial check for same point-origin directly delegates to the sudo_cast<Q> implementation to select the "best" way to do that multiplication, and avoids the addition
• The two code-paths for a change in point origin go as follos:
  • The first path for where the unit gets smaller and thus the number gets larger:
    1. cast to the intermediate representation type using sudo_cast (no-op in the case of floatingpoints). previously, that was a value_cast, but I figured that within a sudo_cast, value_cast is off limit.
    2. add/subtract the change in reference using .point_for; hopefully, the implementation of .point_for is such that this is a single addition and no multiplication
    3. scale the unit/number and
    4. cast the to the final representation type using sudo_cast.
  • The second path for where the unit gets larger and thus the number will get smaller (or stay the same):
    1. cast to the intermediate representation type and
    2. scale the unit/number using sudo_cast. If the intermediate representation type is the same as the source erpresentation and the units remain the same, this is a no_op.
    3. add/subtract the change in reference using .point_for
    4. cast to the final representation type using sudo_cast. No-op for floating-point types.

So I believe this implementation is as efficient as it can be through a careful selection of types. While both code-paths invoke sudo_cast twice, there is always one where the source and target units match and no multiplication is needed. For floating-point, there is also not cast, so that cast is a no-op. Furthermore, the implementation here does never do any arithmetic itself - it delegates the addition to .point_for and the multiplication to sudo_cast<Q>. Personally, I would even prefer to call value_cast instead, because in each of the two code-paths, one of the casts is a pure representation cast, which would be more clear as value_cast<rep>.

[burnpanck]

Wait, now I understand you are indicating that .point_for may potentially change the unit and representation type? In that case, you are of course right, we should replace the .point_for call here with a dedicated implementation.

[mpusz]

However, I'm not sure what you mean by "calculate the entire conversion factor in one step".

I mean something like this:

  if constexpr (std::is_floating_point_v<multiplier_type>) {
      // this results in great assembly
      constexpr auto ratio = val(num) / val(den) * val(irr);
     // use precalculated ratio...
  }
  else // ...

Precalculation of ratio as a constexpr variable ensures that the assembly will include only one multiplication by this constant. The else branch typically results in more assembly instructions.

[mpusz]

@burnpanck, are you going to work on this here? Or should we rebase, merge, and then refactor in the next PR?

[burnpanck]

Sorry, work interfered. Yeah, I intend to do a replacement of point_for by a direct implementation which avoids all further unit conversions. However, I believe the code as-is would be mergeable too. As mentioned before, there should definitely be no regressions in this PR as of now. All unit conversions are as efficient as they can be (they re-use the existing implementation). The current value_cast<QP> is no worse than what you would reasonably implement using the tools given by this library as a user (namely, using point_for). The converting constructor of quantity_point is untouched and slightly worse than this value_cast<QP> concerning overflow or truncation with integer types. Maybe it would make sense to switch the converting constructor to delegate to value_cast<QP> and then merge. We can then postpone performance-improvement of value_cast<QP> to another PR, and perhaps discuss separately when the current converting constructor of quantity_cast should be explicit or not.

[mpusz]

OK, so let's do it this way. I need a few more days to finish the framework cleanup I am doing right now. It there will be no further changes in this PR when I am done, I will merge it as is and provide a new mp-units 2.2 release. After that we will continue this effort in the following PRs targeting the next release. If you will have some time in the upcoming days to work on it and you care for all of the changes to be provided in mp-units 2.2., please feel encouraged to do so.

[burnpanck]

I just added a bit more detail to the documentation, opting for the analogy to the value_cast<...>(q) overloads. While doing that, I re-read the quantity-point documentation, and realised that we also have a converting constructor to quantity_point which has the same power as the new value_cast<QP>. However, it doesn't share the same implementation (potentially overflow when the point-origin offset is large compared to the range of the nested quantity type). It also is conditionally implicit even for integral types, which I believe we should re-evaluate given the potential for overflow.

[mpusz]

Sure, it is great that you have time to look into this in more detail. All of this is too much for me to address in detail at once by working alone. There are still plenty of things to do in the library. I am so happy when someone comes to help me. Thank you!