[ntuple] Cast min/max to the actual type when quantizing/unquantizing in Real32Quant #18629
Conversation
The goal is to use exactly 6.34.00, the first public version to produce the final RNTuple format. If that version has implementation bugs, I think I would rather skip |
|
Right, we cannot magically change 6.34.00...then I'd just backport it to 6.36. |
Test Results 19 files 19 suites 3d 14h 1m 30s ⏱️ Results for commit 8b07973. ♻️ This comment has been updated with latest results. |
|
As it's apparent from the unit test failures (which I'm not sure why I didn't see when I ran them locally the first time), either this change is incorrect or some of our tests are (in regards to the max error they allow in respect to the bit width). |
silverweed
force-pushed
the
ntuple_quant_fixes2
branch
3 times, most recently
from
6002780 to
3515a83
Compare
silverweed
force-pushed
the
ntuple_quant_fixes2
branch
2 times, most recently
from
2150616 to
e0aa335
Compare
The min and max value of the range are now given by the user as `T`
rather than always being `double`, guaranteeing that the outOfRange
check is consistent with the user's expectation (in particular this
guarantees that the given `min` and `max` are always considered
in range - which is not the case if the compared values use different
precisions).
The expectations on the unquantized values are as follows:
- the `min` value will always quantize to 0 and unquantize back to
exactly the original `min` value (as represented by the type `T`
of the original RField used to quantize it);
- the `max` value will always quantize to 1 and unquantize back to
a value that is never larger than the given `max` (as represented
by `T`) and that is within "an epsilon" from it (where "epsilon"
is defined as `max(1, value) * numeric_limits<double>::epsilon()`).
Some internal checks are adjusted to reflect the new expectations.
silverweed
force-pushed
the
ntuple_quant_fixes2
branch
from
e0aa335 to
8b07973
Compare
See the commit message for more details.
This PR should be backported, probably even to 6.34 to make it work with the rntuple validation suite.
Checklist: