alexeigor commented on PR #10958: URL: https://github.com/apache/arrow-rs/pull/10958#issuecomment-5514841029
Thanks @Rich-T-kid — both points addressed. **Benchmark sizes.** Now `[512, 1024, 8192, 65536]` as suggested. The `512` names are unchanged, so those rows stay comparable with historical runs. **Trimming.** The PR goes from `+438 −31` to `+362 −29`. Benchmarks — removed the `infallible numeric cast i32 to i64` group (6 benchmark IDs). The size sweep over the existing primitive cast benchmarks covers the same ground, and unlike a newly added group it can actually be compared against the merge base. That group was the one showing a blank `main` column in the run above, so it was contributing bulk without contributing a comparison. The imports it needed are reverted too, so the import diff is gone. Tests — the end-to-end cast test went from all 34 integer-source pairs to one per conversion class (signed widening, signed to float, unsigned widening, unsigned to wider signed, unsigned to float), and `Float32 -> Float64` folded into it rather than having a test of its own. Four test functions down to three. The reasoning for keeping a sample rather than the matrix: the property test already covers all 35 pairs numerically, so re-enumerating them end to end adds no numeric coverage. The end-to-end test is there to check what `unary` could get wrong — length, validity and array offsets — and a representative sample exercises that identically. I left two things intact deliberately: the predicate's correctness matrix, and the negative control asserting narrowing / unsigned-to-signed-of-the-same-width / signed-to-unsigned still return null under `safe` and an error under `safe: false`. Those are what actually guard against a wrong pair entering the fast path — the negative control catches a bad `u32 -> i32` entry that the older assertions did not. **Also in this push:** a fix for the `MIRI (3)` failure on the previous commit. The `Float32 -> Float64` test compared the converted value bitwise including for NaN; only NaN-ness is guaranteed across a float conversion, and Miri randomises the sign and payload deliberately, so it saw `0xFFF8000000000000` against `0x7FF8000000000000`. It now asserts `is_nan()` for NaN and keeps the bitwise comparison everywhere else, so the signed zero case still means something. Locally: `cargo test -p arrow-cast --lib` (381 passed), `cargo fmt --all -- --check`, `cargo clippy -p arrow-cast --lib --tests -- -D warnings`, `cargo clippy -p arrow --bench cast_kernels --features test_utils -- -D warnings`, and the three tests under `cargo miri test` with `-Zmiri-many-seeds=0..8`. Ready for another look when you have a moment. A re-run of `cast_kernels` would also fill in the new 8192 and 65536 columns on neutral hardware. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
