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] Remove incorrect std::vector::reserve #16069

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

hahnjo
Copy link
Member

@hahnjo hahnjo commented Jul 19, 2024

This was added in commit d950e99 ("[ntuple] use RClusterPool in RNTupleMerger") with no explicit mention, and I believe it's wrong because every column yields exactly one page group that contains all pages for that column.

This was added in commit d950e99 ("[ntuple] use RClusterPool in
RNTupleMerger") with no explicit mention, and I believe it's wrong
because every column yields exactly one page group that contains all
pages for that column.
@hahnjo hahnjo requested a review from silverweed July 19, 2024 13:03
@hahnjo hahnjo self-assigned this Jul 19, 2024
@hahnjo hahnjo requested a review from jblomer as a code owner July 19, 2024 13:03
Copy link
Contributor

@silverweed silverweed left a comment

Choose a reason for hiding this comment

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

You're totally right, thanks for noticing

Copy link

Test Results

    13 files      13 suites   2d 19h 32m 19s ⏱️
 2 652 tests  2 650 ✅ 0 💤 2 ❌
32 658 runs  32 656 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 64f0389.

@hahnjo hahnjo merged commit 5738825 into root-project:master Jul 19, 2024
15 of 18 checks passed
@hahnjo hahnjo deleted the ntuple-merger-reserve branch July 19, 2024 15:18
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.

None yet

2 participants