Skip to content

[hist] Implement initial RAxes #19596

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

Merged
merged 5 commits into from
Aug 14, 2025
Merged

[hist] Implement initial RAxes #19596

merged 5 commits into from
Aug 14, 2025

Conversation

hahnjo
Copy link
Member

@hahnjo hahnjo commented Aug 11, 2025

This first version supports RRegularAxis and RVariableBinAxis and exposes only ComputeTotalNumBins and ComputeGlobalIndex.

@hahnjo hahnjo self-assigned this Aug 11, 2025
@hahnjo hahnjo requested a review from bellenot as a code owner August 11, 2025 07:23
@hahnjo hahnjo added the in:Hist label Aug 11, 2025
Copy link

github-actions bot commented Aug 11, 2025

Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 309d88a.

♻️ This comment has been updated with latest results.

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.

In principle LGTM. Some suggestions.

hahnjo added 2 commits August 14, 2025 11:01
It only takes a single argument, avoid accidental construction from
any std::vector<double>.
hahnjo added 3 commits August 14, 2025 12:08
This first version supports RRegularAxis and RVariableBinAxis and
exposes only ComputeTotalNumBins and ComputeGlobalIndex.
This method will be extensively used during histogram filling, so it
should be fast.
This may cause a minor hit in the microbenchmark (for two dimensions),
but this will likely be within noise together with future changes to
check the argument types for a given axis type.
@hahnjo hahnjo merged commit a80d113 into root-project:master Aug 14, 2025
6 of 27 checks passed
@hahnjo hahnjo deleted the hist-axes branch August 14, 2025 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants