Skip to content

fix(java): round UInt64 values to the nearest float and double - #4196

Open
NUAAwqy wants to merge 1 commit into
apache:mainfrom
NUAAwqy:fix/java-uint64-unsigned-widening
Open

NUAAwqy wants to merge 1 commit into
apache:mainfrom
NUAAwqy:fix/java-uint64-unsigned-widening

Conversation

@NUAAwqy

@NUAAwqy NUAAwqy commented Oct 25, 2026

Copy link
Copy Markdown

Why?

UInt64 documents doubleValue() as the way to read a value that has left the signed long range, and that accessor was one ulp low for one residue class in every binade from 2^53 up: halving the unsigned value first put the odd low bit below the double grid, so it could only nudge the result down instead of breaking the tie. floatValue() was worse — it narrowed through doubleValue(), so a second rounding could land a value exactly on a float midpoint, and the tie then broke by mantissa parity instead of by which side of the midpoint the integer is on. Examples (unsigned inputs):

input old doubleValue() nearest double
2^54 + 3 1.8014398509481984E16 1.8014398509481988E16
2^62 + 513 4.6116860184273879E18 4.6116860184273889E18
input old floatValue() nearest float
2^53 + 2^29 + 1 9.0071993E15 9.0072003E15

In [2^63, 2^64) the float midpoint band is 1,024–1,025 values wide per midpoint, so on the order of 2^33 values in the top binade alone are mis-reported. UInt8/UInt16/UInt32 widen exactly, so UInt64 is the only broken wrapper. Nothing inside the repository calls either accessor, which is why the existing suite cannot see it.

What does this PR do?

Round once, straight to the target format. Non-negative payloads already narrow correctly under JLS 5.1.3, so they become a plain cast. The top unsigned binade [2^63, 2^64) is handled on an integer grid — 2^11 for double, 2^40 for float — whose quotient fits the target mantissa exactly, so the remainder decides the rounding and the quotient's parity decides the tie; the result is the correctly-rounded value by construction, with no second rounding. floatValue() no longer delegates to doubleValue().

Evidence

Differential census against a decimal-parse oracle (Double.parseDouble / Float.parseFloat of the unsigned decimal string, which JLS requires to be correctly rounded), old formula vs new on identical populations:

  • 524,288 consecutive values from every binade start → old doubleValue() 2,046 wrong, new 0
  • 5,000,000 random unsigned values → old doubleValue() 6,157 wrong, new 0
  • 2,389,824 samples across the double and float midpoint bands → old 352 double and 1,050,624 float wrong, new 0 and 0

UnsignedTest gains three tests: literal spot checks (both mis-rounded classes above, plus MAX_VALUE and 2^63, which stay correct), a double sweep comparing against parseDouble across 64 grid points per binade 2^53…2^63 (640 disagreements before the fix), and a float sweep (108 disagreements before). All sweep loops are JDK-8 safe, so the JDK 8/11/17/21/25/26 CI matrix is safe.

  • Red on pristine 8e93633 + the new tests: Tests run: 95, Failures: 3 → green after the fix: Tests run: 95, Failures: 0
  • Full fory-core module: 2349 tests / 1 failure (ExpressionTest.testLiteralSourceRoundTrip, pre-existing — reproduces on the base commit)
  • spotless:check / checkstyle:check pass; ASF license headers preserved; no new files

User-facing change

UInt64.doubleValue() and floatValue() may return a value one ulp different for inputs at or above 2^53; they now return the nearest representable float/double in every case. Nothing in the repository depends on the old results; downstream code comparing exact doubles could.

AI Usage Disclosure

  • substantial_ai_assistance: yes
  • scope: code drafting (fix), tests, PR body drafting
  • affected_files_or_subsystems: java/fory-core/src/main/java/org/apache/fory/type/unsigned/UInt64.java, java/fory-core/src/test/java/org/apache/fory/type/unsigned/UnsignedTest.java
  • ai_review: line-by-line self-review completed (rounding cases checked against IEEE-754 round-to-nearest-even on the 2^11/2^40 grids; red→green confirmed). The §5 two-reviewer AI review loop with persisted artifacts has not been attached yet; it can be run and linked before maintainer review on request.
  • ai_review_artifacts: none persisted yet (see previous line)
  • human_verification: mvn -pl fory-core test -Dtest=UnsignedTest red (3 failures) → green (0); full module 2349 tests / 1 pre-existing failure; decimal-parse census 2,046/6,157/1,050,624 wrong → 0; spotless/checkstyle clean
  • performance_verification: N/A — integer-grid widening replaces a multiply-add; no serialization hot-path or wire-format change
  • provenance_license_confirmation: original work under Apache-2.0; no third-party code introduced; license headers preserved

UInt64 widened its unsigned magnitude with `(double) (value >>> 1) * 2 + (value & 1L)`. Halving
first puts the odd bit below the double grid wherever the spacing exceeds 2, so a residue just
above a midpoint can only round down: 2^54 + 3 gave 1.8014398509481984E16 and 2^62 + 513 gave
4.6116860184273879E18, each one double too low. The class is one residue per two grid points in
every binade from 2^53 up - 2,046 misses in the first 8,192 values of each binade, 6,157 in 5M
random values - and it lands on the very accessor the class javadoc recommends "when the full
unsigned magnitude must be exposed beyond the signed long range".

floatValue() then narrowed that double again, which is wrong on its own account: a value just
above a float midpoint can collapse onto that midpoint in the double step, where ties go to even
mantissa instead of to the side the integer actually sits on. 2^53 + 2^29 + 1 came back as
9.0071993E15 rather than 9.0072003E15, and in [2^63, 2^64) the band around every float midpoint is
1,024 values wide, roughly 2^33 of the space.

Both helpers now round once, straight to the target format. Non-negative longs already narrow
correctly under JLS 5.1.3, and the top binade is a plain grid - 2^11 for double, 2^40 for float -
whose tie is decided by the parity of the quotient.

Evidence: UInt64 from the built module swept against Double/Float.parseFloat of each value's own
unsigned decimal string, which the JLS requires to be correctly rounded. 524,288 consecutive values
taken from every binade start: 2,046 wrong doubleValues before, 0 after. 2,389,824 values across
the double and float midpoint bands: 352 wrong doubleValues and 1,050,624 wrong floatValues before,
0 after. 5,000,000 random unsigned values: 6,157 wrong doubleValues before, 0 after.

The three new UnsignedTest cases keep two of those sweeps in-repo, walking the midpoint classes of
every binade from 2^53 and comparing against the same decimal oracle: 640 and 108 failures before
the fix, 0 after. No existing test called either accessor, and none of the parse/serialize
round-trips go through them, so nothing in the suite could have seen this.

Verified on JDK 17: UnsignedTest 95/95 (3 red before the fix); full fory-core suite
`Tests run: 2349, Failures: 1`, the lone failure being the pre-existing
ExpressionTest.testLiteralSourceRoundTrip, which also fails on main without this change;
spotless:check and checkstyle:check clean, spotless run under JDK 21 as .agents/languages/java.md
requires. No other Java module calls either accessor.

Signed-off-by: Qingyuan Wang <162230229@nuaa.edu.cn>
@NUAAwqy
NUAAwqy requested a review from chaokunyang as a code owner October 25, 2026 13:13

This branch has not been deployed

No deployments
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 this pull request may close these issues.

1 participant