Round exact ties to nearest-even when converting to float8_e8m0fnu - #398
Round exact ties to nearest-even when converting to float8_e8m0fnu#398EylonKrause wants to merge 2 commits into
Conversation
Converting a float/double/bfloat16 to float8_e8m0fnu (the mantissa-less,
exponent-only scale type) rounded every exact tie up instead of to
nearest-even, contradicting the type's round_style (round_to_nearest)
and the RoundBitsToNearestEven routine used for the conversion. For
example 3.0 (an exact tie between 2.0 and 4.0) converted to 4.0 rather
than 2.0.
RoundBitsToNearestEven derives the round-to-even ("last kept") bit as
bit `roundoff` of the mantissa. For a mantissa-less target that bit is
the always-1 implicit bit, so ties always rounded up; the result's true
least-significant bit is the exponent LSB, which the implicit bit carries
into during packing.
Add an overload of RoundBitsToNearestEven that accepts the last-kept bit
explicitly, and pass the exponent LSB for the kToMantissaBits == 0 case.
Targets with mantissa bits are unchanged. 126 of the 254 e8m0 tie points
were previously mis-rounded.
febd39a to
d3cd0b4
Compare
|
Thanks – one complication here is that this PR only touches the NumPy conversion, and does not change the behavior of conversion on-device for programs generated by JAX. Currently both behaviors match: >>> import ml_dtypes
>>> import numpy
>>> np.float32(3.0).astype(ml_dtypes.float8_e8m0fnu)
4
>>> import jax.numpy as jnp
>>> from jax import lax
>>> lax.convert_element_type(jnp.float32(3.0), jnp.float8_e8m0fnu)
Array(4, dtype=float8_e8m0fnu)If we merge this change as-is, it will mean that host-side casting with NumPy would not match device-side casting with JAX/XLA, and that could end up being confusing. I'm curious what @hawkinsp thinks of this – is this behavior something we should seek to change in ml_dtypes and/or XLA? |
|
I think I agree with the PR author that we need to round to nearest even so this is a bug we should fix in both places. |
|
Thanks both! Agreed — round-to-nearest-even is what This PR is self-contained to the ml_dtypes (host/NumPy) conversion; the XLA device lowering is a separate change. A couple of options, whichever you prefer:
Either way works for me — let me know how you'd like to sequence it. |
|
Can you check that this PR works for the lowest binade (i.e. 0x0)? It might be good to add a unit tests: |
|
Good catch — the lowest binade was wrong. The first version read the round-to-even bit after the denormal adjustment had already widened the round position, so ties where the target exponent is clamped to 0 still rounded up ( Fixed it properly: since e8m0 has no mantissa and no subnormals, consecutive encodings are always one power of two apart, so I now round the source fraction about its own binade midpoint (bit I verified it against round-to-nearest-even for every finite positive float32 and every binade tie (lowest through highest) — 0 mismatches. Added your suggested case plus the two neighbours: EXPECT_EQ(rep(0x1.8p-127), 0x00); // tie 2^-127 (0x00, even) / 2^-126 (0x01) -> 0x00
EXPECT_EQ(rep(0x1.4p-127), 0x00); // nearer 2^-127
EXPECT_EQ(rep(0x1.cp-127), 0x01); // nearer 2^-126 |
The first version read the round-to-even bit after widening the round position by the denormal adjustment, so ties in the lowest binade (target exponent clamped to 0) still rounded up -- e.g. 1.5*2^-127 gave 0x01 instead of 0x00. E8M0 has no mantissa and no subnormals: consecutive encodings are always a full power of two apart, so round the source fraction about its own binade midpoint (bit kFromMantissaBits), not about alignment_shift, and break ties toward the even result encoding, whose LSB is the low bit of the truncated (round-toward- zero) biased exponent. Verified against round-to-nearest-even for every finite positive float32 and every binade tie, including the lowest and highest.
2d56dc1 to
728ce42
Compare
4 participants
Summary
Converting a float/double/bfloat16 to
float8_e8m0fnu— the mantissa-less, exponent-only scale type used by the MX formats — rounds every exact tie up instead of to nearest-even, contradicting the type's own rounding mode (std::numeric_limits<float8_e8m0fnu>::round_style == round_to_nearest) and theRoundBitsToNearestEvenroutine used for the conversion.3.0is exactly halfway between the representable values2.0(0x80) and4.0(0x81); ties-to-even should select2.0(even encoding), but the code produces4.0. A sweep of all tie midpoints shows 126 of the 254 are mis-rounded (every tie whose nearest-even value is the smaller one).Cause
RoundBitsToNearestEven(bits, roundoff)derives the round-to-even ("last kept") bit as(bits >> roundoff) & 1. For a mantissa-less targetroundoff == kFromMantissaBits, so that bit is the always-1 implicit leading bit — hence it is always 1 and every tie rounds up. The result's actual least-significant bit is the exponent LSB, into which the implicit bit carries during packing.Fix
Add an overload of
RoundBitsToNearestEventhat takes the last-kept bit explicitly (the existing two-argument form is unchanged and simply forwards the old default). InConvertImpl, forkToMantissaBits == 0, supply the exponent LSB so exact ties round to even. Targets with mantissa bits are entirely unaffected.Testing
Float8E8m0Test.ConvertFromTiesToEvenchecks that ties round to even (and non-ties to nearest).float8 -> float8conversion is unchanged.Disclosure: this contribution was authored with an AI coding assistant (Claude) and reviewed before submission.