Skip to content

fix(core)!: make casts exact at integer and float range limits - #36

Merged
d-v-b merged 4 commits into
zarr-developers:mainfrom
d-v-b:claude/audit-rust-zarrs-1957ae
Oct 29, 2026
Merged

fix(core)!: make casts exact at integer and float range limits#36
d-v-b merged 4 commits into
zarr-developers:mainfrom
d-v-b:claude/audit-rust-zarrs-1957ae

Conversation

@d-v-b

@d-v-b d-v-b commented Oct 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

🤖 AI text below 🤖

This PR fixes seven cast_value correctness bugs where this crate disagreed with the spec and with the zarrs implementation, and two older bugs that an adversarial review turned up. Casts at integer and float range limits, directed rounding of wide integers, and float16 conversions are now exact, and the slice functions no longer write out of bounds.

Correctness fixes (first commit)

# Bug Before → after
1 ±Inf → int with clamp was clamped; the spec requires an error unless scalar_map covers it. Fixed in the scalar path and every SIMD kernel (NaN check is now a NaN/Inf check via x - x). f64 +Inf→u8 clamp: 255 → error
2 The float→int upper bound rounded up (i32::MAX as f32 is 2³¹), so values at 2³¹/2⁶³/2⁶⁴ were silently saturated. The check now uses hi + 1, which is exactly 2^k in every float type. f32 2³¹→i32 (no out_of_range): i32::MAX → error; f64 2⁶⁴→u64 wrap: u64::MAX → 0
3 wrap computed (val - lo).rem_euclid(range) + lo in the source float type, which rounds for large values. It now uses the i64 bit pattern, or rem_euclid(2^64) beyond the i64 domain, both exact. f64 2⁶³+2048→i64 off by 2048 → exact
4 nearest-away used floor(|x|+0.5), which fails on 0.49999999999999994 and on odd integers above 2⁵² (2²³ for f32). It now uses round(). 0.49999999999999994→1 → 0
5 Int→float directed rounding compared against val as f64, which is already rounded. The integer's bits are now rounded directly to the target precision. i64 2⁵³+3→f64 towards-zero: 2⁵³+4 → 2⁵³+2
6 Float→float with a directed mode stepped an overflowing value back to ±MAX. Overflow is now judged on the rounded value: clamp gives ±Inf, no mode gives an error; values that round onto MAX still return MAX. f64 1e39→f32 towards-zero clamp: MAX → inf
7 float16: f64→f16 went through f32 and rounded twice; int→f16 had no overflow check. 1+2⁻¹¹+2⁻⁴⁰→1.0 → 1.0009766; i32 70000→f16: inf → error

Review follow-ups (second commit)

Four adversarial reviewers each checked one area against an independent exact reference model: every f16 bit pattern, all 2³² f32→f16 casts, and about 10M f64→f32 edge points. They found no mismatch in the new numerics. The second commit fixes what they did find:

  • Out-of-bounds write (older bug): the public slice functions never checked src.len() == dst.len(), and the SIMD kernels write dst through raw pointers sized by src, so a short dst was undefined behaviour from safe code. All four slice functions now return CastError::LengthMismatch before writing.
  • wasm32 tie rounding (older bug): the wasm32 clamp kernels' scalar tails rounded nearest-even ties away from zero (2.5 → 3).
  • Error reporting: float-target overflow is now CastError::FloatOutOfRange, without the wrap hint that the spec forbids for floats. OutOfRange reports the integer target's exact bounds and the unrounded input, instead of messages like Value 2147483648 out of range [-2147483648, 2147483648].
  • Error order: the f64→i32 kernel used when out_of_range is unset hands off to the scalar path at the first bad vector. [3e9, inf] now reports the out-of-range 3e9, as the scalar path does.
  • Cleanup: removed the now-unused CastFloat::rem_euclid, fixed stale docs and Python docstrings, and grouped the binding helpers' arguments into CastOptions (it failed clippy::too_many_arguments).

Performance (third commit)

CodSpeed flagged i64→f32 (−29%) and f64→f32 (−10%) towards-zero after the exactness fixes. The third commit recovers this:

  • The int→float and float→float slice loops now pick the rounding mode once and run a loop specialised to it.
  • Exactly representable integers skip the slow rounding path.
  • Targets an integer cannot overflow skip the overflow test.
  • The float→float overflow decision moved into a cold function.

In an interleaved A/B harness on aarch64, all affected cases are now within run-to-run noise of main.

Breaking changes

  • IntToFloatConfig gains an out_of_range field.
  • CastInt now requires num_traits::FromPrimitive.
  • CastError gains FloatOutOfRange and LengthMismatch, and OutOfRange's lo/hi are now i128.
  • CastFloat::rem_euclid is removed.
  • CastFloat::round_with_mode(NearestAway) output changes for f32/f64 (fix 4).
  • ±Inf → int with clamp now errors instead of clamping.
  • Mismatched slice lengths now error, where the scalar path previously truncated silently.

Testing

  • Rust: just test-rust passes (50 tests by default, 65 with float16). just fmt-check, just lint, the float16 clippy run and binding-crate clippy are all clean.
  • wasm32: the full Rust suite passes under Node's WASI runtime, both with SIMD128 (wasm kernels) and without it (generic kernels).
  • Python: just test-python passes (156 tests).
  • Regression tests: each fix has one. The new SIMD-vs-scalar agreement test fails on the first commit and passes now.
  • Not run locally: the x86_64 AVX/AVX2 kernels (no x86 target on the agent's machine). CI is the first execution of those edits.
Release and bookkeeping notes
  • Versions are unchanged (0.4.2). Given the breaking changes, the next release should be 0.5.0.
  • Still open and left for later: wrap is not rejected up front for float targets; f16 next_up(+Inf) returns NaN (no cast reaches it); dst contents after an error differ between SIMD and scalar paths; scalar_map matches -0.0 against a 0.0 key (spec is ambiguous).
  • Pre-existing lints outside CI's scope remain: wasm32 needless_return in simd/mod.rs, and two clippy errors in examples/zarrs_integration.rs.
  • A copy of this PR on the fork (fix(core)!: make casts exact at integer and float range limits d-v-b/cast-value.rs#2) predates the review fixes.

🤖 Generated with Claude Code

@d-v-b
fix(core)!: make casts exact at integer and float range limits
24d0f4e 
Fix seven correctness bugs found by auditing against the zarrs
cast_value implementation:

- float->int: reject +/-Inf in every out_of_range mode (scalar and SIMD
  kernels), as the spec requires unless scalar_map covers it
- float->int: compare against an exact exclusive upper bound, so values
  at 2^31/2^63/2^64 are no longer saturated silently
- float->int wrap: reduce modulo 2^N exactly via i64 bits or
  rem_euclid(2^64) instead of the rounding (val - lo) arithmetic
- nearest-away: use round() instead of floor(|x| + 0.5)
- int->float: round the integer's bits directly to the target precision,
  instead of comparing against an already-rounded f64
- float->float: treat a value whose rounded magnitude exceeds the finite
  range as out of range in every rounding mode (clamp -> +/-Inf)
- float16: convert f64 directly to f16 (single rounding), and range-check
  int->f16 casts

BREAKING CHANGE: IntToFloatConfig gains an `out_of_range` field;
CastInt now requires num_traits::FromPrimitive; +/-Inf -> int with clamp
now errors instead of clamping.

Assisted-by: ClaudeCode:claude-opus-5-5
@codspeed

codspeed Bot commented Oct 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will improve performance by 15.9%

1 warning

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

⚡ 6 improved benchmarks
✅ 45 untouched benchmarks
⏩ 21 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ f64_to_f32/towards_zero[1024] 20.9 µs 17.7 µs +18.17%
⚡ f64_to_f32/towards_zero[64000] 1.2 ms 1.1 ms +17.6%
⚡ f64_to_f32/towards_zero[1000000] 19.5 ms 16.6 ms +17.59%
⚡ f64_to_i32/no_oor[1000000] 7.8 ms 6.8 ms +14.32%
⚡ f64_to_i32/no_oor[64000] 497.9 µs 435.6 µs +14.31%
⚡ f64_to_i32/no_oor[1024] 9.2 µs 8.1 µs +13.5%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.

Comparing d-v-b:claude/audit-rust-zarrs-1957ae (38dae71) with main (b28610f)

Open in CodSpeed

Footnotes

  1. 21 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

d-v-b added 2 commits October 29, 2026 12:49
@d-v-b
fix(core)!: address adversarial review of exact-cast changes
2027e07 
- reject src/dst length mismatches in every slice function with
  CastError::LengthMismatch; the SIMD kernels wrote past a short `dst`
  through raw pointers (undefined behaviour reachable from safe code)
- fix nearest-even ties in the wasm32 clamp kernels' scalar tails, which
  rounded half away from zero
- report float-target overflow as the new CastError::FloatOutOfRange
  (no misleading `wrap` hint); OutOfRange now carries the integer
  target's exact i128 bounds and the unrounded input
- make the f64->i32 check kernels hand off to the scalar path on the
  first bad vector, so errors match the scalar path in variant and order
- remove the now-unused CastFloat::rem_euclid
- group the Python binding helpers' shared arguments into CastOptions
  (fixes clippy::too_many_arguments)
- fix stale docs and Python docstrings; add tests for length mismatch,
  error bounds, SIMD-vs-scalar agreement, the generic kernels, and
  int->float16 overflow in the Python `into` variant

BREAKING CHANGE: CastError gains FloatOutOfRange and LengthMismatch,
and OutOfRange's lo/hi are now i128; CastFloat::rem_euclid is removed.

Assisted-by: ClaudeCode:claude-opus-5-5
@d-v-b
perf(core): recover directed-rounding throughput for float targets
e1c43ad 
CodSpeed flagged i64->f32 (-29%) and f64->f32 (-10%) towards-zero after
the exact-cast fixes. Recover it by:

- hoisting the rounding-mode match out of the int->float and
  float->float slice loops, so each loop is specialised to one mode
- skipping int->float directed rounding when the integer's significant
  bit span fits the target significand (the `as` cast is then exact)
- skipping the int->float overflow test for targets an integer cannot
  overflow (constant-folded; only f16 keeps it)
- moving the float->float exact overflow decision into a cold function
  behind a branch-free infinity test

Assisted-by: ClaudeCode:claude-opus-5-5
@d-v-b d-v-b mentioned this pull request Oct 29, 2026
Open
@d-v-b
perf(core): cheapen the int->float exactness test
38dae71 
CodSpeed still measured i64->f32 towards-zero 16% slower: the
leading/trailing-zero span test costs several instructions on x86.
Detect exactness the way the original code did, comparing the integer
and the nearest-even result in f64, but only trust it while the f64
value is below 2^53 (always, for sources of at most 53 bits), where f64
holds the integer exactly. The exact-value loop body is now one
instruction shorter than on main (aarch64 asm: 11 vs 12).

Assisted-by: ClaudeCode:claude-opus-5-5
@d-v-b
d-v-b merged commit a149383 into zarr-developers:main Oct 29, 2026
10 checks passed
This was referenced Oct 29, 2026
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

breaking change fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@d-v-b