feat(pyamber): canonicalize NaN hashes - #8252
Conversation
Backport auto-label reportThis
|
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8252 +/- ##
============================================
- Coverage 93.93% 93.90% -0.04%
+ Complexity 4762 4761 -1
============================================
Files 1191 1191
Lines 48308 49053 +745
Branches 5377 5377
============================================
+ Hits 45380 46061 +681
- Misses 1494 1557 +63
- Partials 1434 1435 +1
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 397 | 0.242 | 23,640/33,120/33,120 us | 🔴 +12.2% / 🔴 +112.6% |
| 🔴 | bs=100 sw=10 sl=64 | 888 | 0.542 | 111,188/140,412/140,412 us | 🔴 +11.9% / 🔴 +30.9% |
| 🔴 | bs=1000 sw=10 sl=64 | 1,063 | 0.649 | 932,746/1,061,157/1,061,157 us | 🔴 +5.2% / 🟢 -5.0% |
Baseline details
Latest main 50321e4 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 397 tuples/sec | 449 tuples/sec | 787.98 tuples/sec | -11.6% | -49.6% |
| bs=10 sw=10 sl=64 | MB/s | 0.242 MB/s | 0.274 MB/s | 0.481 MB/s | -11.7% | -49.7% |
| bs=10 sw=10 sl=64 | p50 | 23,640 us | 21,063 us | 12,593 us | +12.2% | +87.7% |
| bs=10 sw=10 sl=64 | p95 | 33,120 us | 33,405 us | 15,579 us | -0.9% | +112.6% |
| bs=10 sw=10 sl=64 | p99 | 33,120 us | 33,405 us | 18,786 us | -0.9% | +76.3% |
| bs=100 sw=10 sl=64 | throughput | 888 tuples/sec | 956 tuples/sec | 1,008 tuples/sec | -7.1% | -11.9% |
| bs=100 sw=10 sl=64 | MB/s | 0.542 MB/s | 0.583 MB/s | 0.615 MB/s | -7.0% | -11.9% |
| bs=100 sw=10 sl=64 | p50 | 111,188 us | 104,099 us | 100,701 us | +6.8% | +10.4% |
| bs=100 sw=10 sl=64 | p95 | 140,412 us | 125,455 us | 107,244 us | +11.9% | +30.9% |
| bs=100 sw=10 sl=64 | p99 | 140,412 us | 125,455 us | 116,122 us | +11.9% | +20.9% |
| bs=1000 sw=10 sl=64 | throughput | 1,063 tuples/sec | 1,082 tuples/sec | 1,042 tuples/sec | -1.8% | +2.0% |
| bs=1000 sw=10 sl=64 | MB/s | 0.649 MB/s | 0.661 MB/s | 0.636 MB/s | -1.8% | +2.0% |
| bs=1000 sw=10 sl=64 | p50 | 932,746 us | 920,869 us | 981,959 us | +1.3% | -5.0% |
| bs=1000 sw=10 sl=64 | p95 | 1,061,157 us | 1,008,231 us | 1,023,080 us | +5.2% | +3.7% |
| bs=1000 sw=10 sl=64 | p99 | 1,061,157 us | 1,008,231 us | 1,051,697 us | +5.2% | +0.9% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,503.94,200,128000,397,0.242,23639.65,33119.76,33119.76
1,100,10,64,20,2252.77,2000,1280000,888,0.542,111187.57,140411.64,140411.64
2,1000,10,64,20,18819.39,20000,12800000,1063,0.649,932746.13,1061157.07,1061157.07|
I am converting this PR to a draft: please follow our PR template and mark it ready for review. |
|
The description now follows the current template, and this is ready for review. |
What changes were proposed in this PR?
Python tuple hashing now canonicalizes every NaN bit pattern before applying the Java double hash. Finite values, infinities, and signed zero remain unchanged.
Any related issues, documentation, discussions?
Closes #8251
How was this PR tested?
Direct reproduction after the fix:
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Codex