Skip to content

Conversation

@ferdymercury
Copy link
Collaborator

@ferdymercury ferdymercury commented Oct 26, 2025

This is related to #20174 and the discussion in #20176

fyi @aapo-kossi

Should we completely disallow infinite bin edges, since it violates a bit the assumptions of under and overflow bins? (As well as complicates the implementation of GetBinCenter, Low and Up Edge, etc). And the workaround is quite easy, just setting 1e30 / 1-e30 as variable bin edge.

Or should we only allow inf for variable bin edges, and not for fixed binwidth since there it does not make sense?

Closes #20174

@ferdymercury ferdymercury marked this pull request as ready for review October 26, 2025 10:39
@ferdymercury ferdymercury requested a review from pcanal as a code owner October 26, 2025 12:02
@ferdymercury ferdymercury changed the title [hist] raise error of badly sorted edges as with variable binwidth [hist] raise error of badly sorted edges as with variable binwidth, and check for inf/nan Oct 26, 2025
@ferdymercury ferdymercury requested a review from dpiparo as a code owner October 26, 2025 12:55
@ferdymercury ferdymercury marked this pull request as draft October 26, 2025 13:02
@github-actions
Copy link

github-actions bot commented Oct 26, 2025

Test Results

    21 files      21 suites   3d 15h 51m 49s ⏱️
 3 705 tests  3 658 ✅ 0 💤 47 ❌
75 919 runs  75 872 ✅ 0 💤 47 ❌

For more details on these failures, see this check.

Results for commit 7d59400.

♻️ This comment has been updated with latest results.

@ferdymercury ferdymercury changed the title [hist] raise error of badly sorted edges as with variable binwidth, and check for inf/nan [hist] raise error of when axis edges are inf/nan Oct 27, 2025
@ferdymercury ferdymercury marked this pull request as ready for review October 27, 2025 07:35
@ferdymercury ferdymercury changed the title [hist] raise error of when axis edges are inf/nan [hist] raise error of when axis edges are inf/nan in fixed-width histos Oct 27, 2025
@hahnjo
Copy link
Member

hahnjo commented Oct 27, 2025

@ferdymercury can you please remove the histv7 changes from this PR? It's on my list to think about what to do with non-finite arguments, but I would prefer a "global" plan of what do with them in the "new" histograms, which may or may not be the same as the "old" ones.

Also in terms of contribution style, can you please try to at least compile-test your changes before pushing and running regression tests at least once? Every action generates emails and notifications which gets annoying if there are 10 within 2 minutes...

@ferdymercury ferdymercury changed the title [hist] raise error of when axis edges are inf/nan in fixed-width histos [hist] raise error of when axis edges are inf/nan in histos Oct 27, 2025
@ferdymercury ferdymercury changed the title [hist] raise error of when axis edges are inf/nan in histos [hist] raise error of when axis edges are inf/nan Oct 27, 2025
@ferdymercury ferdymercury added this to the 6.38.00 milestone Oct 28, 2025
Copy link
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.

Hello @ferdymercury,

that's a good idea! I added suggestions to directly test std::isfinite() to reduce the amount of lines.

ferdymercury and others added 2 commits October 29, 2025 15:52
Co-authored-by: Stephan Hageboeck <stephan.hageboeck@cern.ch>
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.

TH1 and derivates projections produce incorrect results for histograms with infinite bin edges

3 participants