Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ntuple] Improve RNTupleProcessor entry number bookkeeping #17290

Merged

Conversation

enirolf
Copy link
Contributor

@enirolf enirolf commented Dec 17, 2024

This PR improves the bookkeeping of entries in the RNTupleProcessor. Most importantly, a clearer distinction is made between the current entry number (relative to the currently open page source) and the number of entries processed. There was an incorrectness where this value was equal, whereas (assuming we linearly process one page source) fNEntriesProcessed should be higher than fLocalEntryNumber (e.g., after loading the first entry, fNEntriesProcessed == 1, and fLocalEntryNumber == 0).

In the process, some variables have been renamed to better capture their meaning.

As a consequence of this refactoring, the RNTupleSingleProcessor can now also handle empty ntuples.

Related to #17132.

More clearly distinguishes between the number of entries processed and
the entry number (relative to the open page source) currently loaded.
@enirolf enirolf self-assigned this Dec 17, 2024
@enirolf enirolf requested a review from jblomer as a code owner December 17, 2024 16:49
Copy link

Test Results

    18 files      18 suites   4d 3h 28m 53s ⏱️
 2 678 tests  2 676 ✅ 0 💤 2 ❌
46 484 runs  46 482 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 64bd2d1.

Copy link
Contributor

@jblomer jblomer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@enirolf enirolf merged commit 1e57428 into root-project:master Dec 18, 2024
18 of 21 checks passed
@enirolf enirolf deleted the ntuple-processor-entry-bookkeeping branch February 3, 2025 10:56
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