[hist] in TTree::Draw, last bin should include vmax values

[ferdymercury]

This Pull request:

Changes or fixes:

Fixes https://root-forum.cern.ch/t/bug-or-feature-in-ttree-draw/62862

Rounding issue reproducer::

TH1F h("h","h", 100, -1000, 1e31*std::nextafter(float(0),INFINITY)); h.Fill(-999); h.Fill(0); h.GetBinContent(100)

changing to 1e32 makes it work.

It comes down to this line in TAxis:

bin = 1 + int (fNbins*(x-fXmin)/(fXmax-fXmin) );

so:

int(100*1000./(1000.+1e-13)) is 99 whereas int(100*1000./(1000.+1e-14)) is 100

Checklist:

• [x] tested changes locally
• [x] updated the docs (if necessary)

[github-actions[bot]]

Test Results

    18 files      18 suites   4d 21h 46m 10s ⏱️  2 715 tests  2 715 ✅ 0 💤 0 ❌ 47 172 runs  47 172 ✅ 0 💤 0 ❌

Results for commit 0d576839.

:recycle: This comment has been updated with latest results.

[ferdymercury]

I would propose merging this soonish, since it's silently hiding data. (Hiding them in the overflow)

[hageboeck]

I would propose merging this soonish, since it's silently hiding data. (Hiding them in the overflow)

Hello @ferdymercury,

Thanks for your persistence on this issue! 🙂

I agree that it's an annoying "feature", so it needs to be fixed. I looked again today, and believe I found the underlying cause: OptimizeLimits removes the first/last bin from the range based on a slightly arbitrary distance criterion. By adding a check that min and max are within the axis range, I managed to fix the issue. You can see the relevant change in #19506. There is a good argument to doing it in that function, because the check I added uses the same floating-point arithmetic as the computation of the axis range, so it's guaranteed that min/max fall within the valid range (unless I didn't get < vs <= right, please have a look!). #19605 is in draft for the moment, because I plan to add a test also.

[ferdymercury]

Thanks Stephan for looking into it and proposing an alternative PR!