Skip to content

[hist] Implement RHistStats::Taint() and RHist{,Engine}::SetBinContent#21355

Merged
hahnjo merged 5 commits intoroot-project:masterfrom
hahnjo:hist-set-taint
Mar 25, 2026
Merged

[hist] Implement RHistStats::Taint() and RHist{,Engine}::SetBinContent#21355
hahnjo merged 5 commits intoroot-project:masterfrom
hahnjo:hist-set-taint

Conversation

@hahnjo
Copy link
Copy Markdown
Member

@hahnjo hahnjo commented Feb 23, 2026

As discussed during the last PPP, calling RHist::SetBinContent will taint the global histogram statistics.

@hahnjo hahnjo self-assigned this Feb 23, 2026
@hahnjo hahnjo added the in:Hist label Feb 23, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 23, 2026

Test Results

    22 files      22 suites   3d 7h 6m 15s ⏱️
 3 835 tests  3 828 ✅ 0 💤 7 ❌
76 599 runs  76 583 ✅ 9 💤 7 ❌

For more details on these failures, see this check.

Results for commit 1c7a6ce.

♻️ This comment has been updated with latest results.

@hahnjo
Copy link
Copy Markdown
Member Author

hahnjo commented Mar 16, 2026

gentle ping :-)

If a dimension is disabled, it should not be possible to retrieve its
quantities.
Copy link
Copy Markdown
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

Comment thread hist/histv7/inc/ROOT/RHistStats.hxx Outdated
Copy link
Copy Markdown
Member

@hageboeck hageboeck left a comment

Choose a reason for hiding this comment

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

LGTM, but the logic_error could explain a bit more how to avoid tainting.

Comment thread hist/histv7/inc/ROOT/RHistStats.hxx Outdated
hahnjo added 4 commits March 25, 2026 08:29
This will be used to taint the global statistics if the user modifies
bin contents explicitly or slices histograms without preserving all
entries, for example.
The implementation choice is to allow modifications without extra
checks / performance penalty, and throw on read access.
This needs one indirection to construct the std::array of indices
with the right (number of) elements.
Also remove Internal::SetBinContent that is not needed anymore.
It will taint the histogram statistics.
@hahnjo hahnjo merged commit 571d919 into root-project:master Mar 25, 2026
28 of 31 checks passed
@hahnjo hahnjo deleted the hist-set-taint branch March 25, 2026 09:34
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