Implement variable rebinning 2-D histogram classes (ROOT-5224) - #22670
Merged
Conversation
1 task
Test Results 22 files 22 suites 3d 15h 30m 0s ⏱️ For more details on these failures, see this check. Results for commit c11ec94. ♻️ This comment has been updated with latest results. |
Implemented for TProfile2D as well.
The variable-bin code path introduced for ROOT-5224 reinterpreted the group parameters but never adapted the content-filling loops, which still advanced through the old bins with a constant stride. As a result the entire histogram content was accumulated into a single bin, and because the new bin counts were computed before the parameter reinterpretation, the loops also indexed the new histogram with a stale stride, writing out of bounds. Rebuild the merging around per-axis old-to-new bin maps computed from the old bin centers with TAxis::FindFixBin, shared in a new internal header. This maps every old cell (including under- and overflows) to the new cell containing its center, works for constant groups and for user-provided edges alike, and therefore also supports passing bin edges for only one of the two axes, which previously fell through to the fixed-width code path and silently ignored the user's edges. Like TH1::Rebin, warn when a new bin edge does not line up with an old one. Also restore the ResetStats() call for the case where the group count does not divide the number of bins: the statistics tail copied from TH1::Rebin relies on SetBinContent zeroing fTsumw, but this code uses UpdateBinContent, so the rebinned histogram kept its stale pre-rebin statistics. 🤖 Done with the help of AI
The variable-bin code path had the same broken constant-stride merging loops as TH2::Rebin2D, and in addition the constant-group branch passed the null xbins/ybins function parameters instead of the computed edge arrays to SetBins, so rebinning any TProfile2D with variable-width axes crashed with a null-pointer dereference in TAxis::Set. Rewrite the merging with the shared per-axis bin-map helpers, which also replaces the hand-written under-/overflow bookkeeping and makes single-axis variable rebinning work. Validate that a new name is given when bin edges are passed, as in TH1::Rebin. 🤖 Done with the help of AI
The rebinned histograms are validated against reference histograms created directly with the target binning and filled with the same weighted pseudo-data, including under-/overflow entries, bin errors, profile bin entries and the global statistics. Also covered: the constant-group rebinning of a variable-width TProfile2D (which used to crash), the statistics recomputation when the group count does not divide the number of bins, and the diagnostics for a missing new name and for misaligned bin edges. 🤖 Done with the help of AI
Consolidate the per-axis setup of Rebin2D (validation, computation of
the new number of bins, divider warning, axis truncation for
non-dividing group counts, axis definition and old-to-new bin map)
into a shared SetupRebinnedAxis helper, removing the diverging
duplicated preambles of TH2 and TProfile2D.
Behavior fixes on top of the consolidation:
- Reject an empty newname (not only a null one) when bin edges are
passed: Clone("") would silently register a second histogram under
the original name.
- Check the misaligned-edge warning per new bin edge against the old
axis instead of per old-bin-group: the previous check could not fire
for old bins mapped to the flow bins or for new bins that receive no
old bin center.
- Base the under-/overflow "will not be used" warnings on the content
of the full flow row/column instead of single corner cells, whose
1D-style indexing was wrong for 2D arrays, and emit them for
TProfile2D too.
- Drop the stats save/restore in TH2::Rebin2D, which was copied from
TH1::Rebin where it compensates for SetBinContent zeroing fTsumw;
with UpdateBinContent it was redundant and could bake in stats
computed from the old bin centers when fTsumw was zero. Only
ResetStats() for a truncating rebin is needed.
- Drop the SetCanExtend(kNoAxis) guard for the same reason: nothing in
the rewritten flow can extend an axis.
- Give TH2::Rebin2D the same no-op early return for
nxgroup == nygroup == 1 that TProfile2D has.
- Unify the divider warning text between TH2 and TProfile2D.
🤖 Done with the help of AI
Fold the TH2::Rebin forwarding check into the single-axis test as a direct comparison against the equivalent Rebin2D call, replace the variable-to-variable TH2 test (which exercised the same code paths as the both-axes test) with a constant-group rebin of a variable-width TH2 that was not explicitly covered before, and merge the two newname guard checks into one diagnostics scope. 🤖 Done with the help of AI
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #5280 that could not be tested by the CI because the PR branch was called
masterImplement variable rebinning 2-D histogram classes (ROOT-5224)
Implemented for TProfile2D as well
TH2::Rebin2D() and TProfile2D::Rebin2D() now accept optional arrays of new bin edges per axis, like TH1::Rebin() does for one-dimensional histograms. For an axis with a bin-edges array, the group parameter is the new number of bins and the array must hold that many low edges plus the upper edge of the last bin; an axis without an array keeps the existing constant-group rebinning, so the two modes can be mixed. TH2::Rebin() with a bin-edges array now rebins the x-axis with the TH1::Rebin() conventions instead of returning an error.
The merging is implemented with per-axis maps from old to new bins, shared between TH2 and TProfile2D in the internal header hist/hist/src/Rebin2DHelpers.h: every old cell, including under- and overflows, is added to the new cell that contains its bin center, and old bins outside the new axis range end up in the flow bins. Like TH1::Rebin(), a warning is emitted when a new bin edge does not line up with an old bin edge, and a new name is required when bin edges are passed.
The rebinned histograms are validated in a new gtest against reference histograms created directly with the target binning and filled with the same weighted pseudo-data, comparing bin contents, errors, flow bins, profile bin entries and the global statistics. The tests also cover constant-group rebinning of variable-width TH2 and TProfile2D histograms and the statistics recomputation when the group count does not divide the number of bins.