Skip to content

[ntuple] Use stored bit width when recompressing merged pages - #23752

Open
Elvand-Lie wants to merge 1 commit into
root-project:masterfrom
Elvand-Lie:fix/ntuple-merger-recompress-low-precision
Open

Elvand-Lie wants to merge 1 commit into
root-project:masterfrom
Elvand-Lie:fix/ntuple-merger-recompress-low-precision

Conversation

@Elvand-Lie

@Elvand-Lie Elvand-Lie commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

This Pull request:

Fix recompression of truncated and quantized float pages during RNTuple merging, including hadd -f1 when the inputs use a different compression.

Changes or fixes:

Source column elements used their default bit width when calculating packed page sizes. Apply the column descriptor's stored bit width before recompression. The pages stay packed, so no value-range setup is needed.

Add one regression next to the compression tests: merge both 20-bit fields from compression 505 to 101 and compare every value against the source file's stored values.

Validation

Base: upstream master 7494cd79d74531ae35ca4831f0843b345865b3c9.

  • Before the fix, the regression fails with zip buffer too short (exit 1).
  • Built the ntuple and ntupleutil test targets with -j3; full ntuple_merger: 341 tests passed.
  • RNTuple and hadd CTest subset, excluding largefile, largevector and tutorial tests:
100% tests passed, 0 tests failed out of 51
Total Test time (real) = 101.38 sec
  • hadd -f1 out.root in.root: master exits 1; fixed build exits 0, with all 1000 entries matching both stored source fields on readback.
  • Changed-line clang-format 18.1.8 and whitespace checks pass.

AI assistance: Codex (gpt-6.1-sol) implemented and tested this change using defect analysis from Claude Code (opus). Review stays with author before a push.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes low-precision page recompression during RNTuple merging.

Apply the source column's stored bit width before computing packed page
sizes during recompression. This allows truncated and quantized float
pages to be merged with a different output compression.

Compare both fields against their stored source values in a regression.

Assisted-by: codex:gpt-6.1-sol
Assisted-by: claude-code:opus
@silverweed

Copy link
Copy Markdown
Contributor

Thanks. I created #23755 on top of your changes as a more general solution to the issue which should prevent this kind of bug going forward.

@github-actions

Copy link
Copy Markdown

Test Results

    20 files      20 suites   2d 22h 24m 21s ⏱️
 3 866 tests  3 863 ✅ 0 💤 3 ❌
69 527 runs  69 523 ✅ 0 💤 4 ❌

For more details on these failures, see this check.

Results for commit ff9d65f.

@Elvand-Lie

Copy link
Copy Markdown
Contributor Author

@silverweed Cool. Should I close this one, or would you rather merge it first?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants