Repository navigation
Conversation
…rmal Float16.floatToFloat16Bits flushes a value to zero when the subnormal rounding shift reaches 24 bits, but the shift only becomes a guaranteed flush one bit later than that. At netShift == 24 the round and sticky bits still decide, and every float strictly between 2^-25 (the midpoint between zero and 2^-24) and 2^-24 is closer to 2^-24 than to zero, so it has to round up to MIN_VALUE. The guard as written drops all of them. An exhaustive sweep of every 2^32 float32 patterns against java.lang.Float.floatToFloat16 (JDK 20+) finds 16777214 disagreements, and every one of them is this case: the 2^23 - 1 patterns with a non-zero mantissa in the 2^-25 binade, for both signs. Each one encodes as a signed zero instead of 0x0001/0x8001, so a value that is representable in float16 is silently lost. The rounding rule that the conversion must follow is the one the docs already state for the JavaScript codec: round to nearest, ties to the even least-significant bit. Relaxing the guard to netShift > 24 makes the routine agree with the JDK conversion for all 2^32 patterns; the remaining 32766 differences are signaling-NaN payload canonicalisation, which is unchanged by this commit. Nothing else is affected. The flush that the guard still performs for netShift > 24 is what keeps the rounding path out of the shift-count masking traps (netShift - 1 == 32 would otherwise index roundBit and sticky with a masked shift), and the existing Float16Test suite never covered this interval because no float16 bit pattern decodes into it. Verification: - red: Float16Test 20 tests, 2 failures, sweep test reports expected [0] but found [16777214] - green: Float16Test 20/20 after the guard change - full fory-core suite: 2348 tests, 1 failure (ExpressionTest.testLiteralSourceRoundTrip, reproduces without this change) - exhaustive differential sweep against Float.floatToFloat16 over all 2^32 inputs: 0 mismatches; Float16.toFloat over all 2^16 inputs: 0 mismatches Signed-off-by: Qingyuan Wang <162230229@nuaa.edu.cn>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why?
Float16.toBits(3.0e-8f)currently returns0x0000. Every float strictly between 2^-25 and 2^-24 is above the midpoint between zero and the smallest float16 subnormal, so round-to-nearest-even — the rounding rule the project documents for float16 — must place it at0x0001. Instead the conversion's flush-to-zero guard fires one binade too early and drops the whole interval to signed zero. Measured over the whole input space: 16,777,214 of the 2^32 float32 bit patterns are wrong (the entire 2^-25 binade, both signs) — e.g.toBits(4.4702e-8f)→0x0000instead of0x0001,toBits(-3.0e-8f)→0x8000instead of0x8001.What does this PR do?
floatToFloat16Bitsshifts the 24-bit float significand onto the subnormal grid and rounds with a round/sticky correction. ThenetShift >= 24shortcut declares any shift of 24 a guaranteed flush, but atnetShift == 24the correction still has work to do: the exact tie2^-25rounds down to zero (unchanged behaviour, and correct under ties-to-even), while anything above it rounds up toMIN_VALUE. The guard becomesnetShift > 24, which also keeps the very large shifts — where the round/sticky indexing would wrap on Java's masked shift counts — inside the flush branch. Decode (toFloat) is untouched.Evidence
java.lang.Float.floatToFloat16(JDK 20+) over all 2^32 float32 inputs: 16,777,214 disagreements before the fix — all of them this flush, both signs, every pattern of the 2^-25 binade except the exact 2^-25 tie — and 0 after. NaN payload canonicalisation differs from the JDK in 32,766 patterns before and after alike (sNaN → qNaN), so NaN behaviour is untouched.Float16Testgains two tests: a boundary test (the0x33000000tie and the largest float below it stay zero;0x33000001,3.0e-8f,4.4702e-8f,0x337FFFFF, negatives andparse("3e-8")all give0x0001/0x8001) and a sweep over all 2^23−1 mantissas of the 2^-25 binade for both signs asserting zero flushes. Red run: 2 failures; green run: 0.fory-coremodule: 2348 tests, 1 failure —ExpressionTest.testLiteralSourceRoundTrip, which reproduces on pristine8e93633with this change reverted (pre-existing, unrelated package).mvn -pl fory-core spotless:checkandcheckstyle:checkpass (spotless run under JDK 21, per.agents/languages/java.md).Float.floatToFloat16— so the JDK 8/11/17/21/25/26 CI matrix is safe.User-facing change?
No public API or binary-protocol change: values that used to serialize as zero now serialize as the subnormal they should have been. Existing float16 payloads are unaffected because no float16 encoding decodes into that interval, so the xlang round-trip suites and
docs/specification/**are unaffected — this is a Java-only rounding bug in a value range no other encoding produces.AI Usage Disclosure
java/fory-core/src/main/java/org/apache/fory/type/Float16.java,java/fory-core/src/test/java/org/apache/fory/type/Float16Test.javamvn -pl fory-core -am test -Dtest=Float16Testred (2 failures) → green (0); full module 2348 tests / 1 pre-existing failure reproduced on the base commit; exhaustive 2^32 differential sweep 16,777,214 → 0; spotless/checkstyle clean