Fix ROOT7 bin coordinate queries (and axis growth logic while I'm at it) #4688
Conversation
|
Starting build on |
|
Build failed on ROOT-performance-centos7-multicore/default. Failing tests: |
|
Build failed on ROOT-ubuntu16/nortcxxmod. Failing tests: |
|
Build failed on ROOT-fedora30/cxx14. Failing tests: |
|
Build failed on ROOT-fedora27/noimt. Failing tests: |
|
Ping @Axel-Naumann. Clearly I have nothing to do with these test failures, so what should I do next? |
|
Build failed on ROOT-fedora29/python3. Failing tests: |
|
Build failed on mac1014/cxx17. Failing tests: |
|
Build failed on ROOT-ubuntu18.04-i386/cxx14. Failing tests: |
|
Build failed on windows10/cxx14. |
|
Regarding Windows: we have a bug in our CI infra that forces you to fork roottest for the Windows build to succeed. Could you do that, please? |
|
These failures are indeed unrelated (and should now be fixed in master). |
|
Starting build on |
| @@ -116,6 +116,18 @@ TEST(HistImplBinning, EquiDist2D) { | |||
| RAxisEquidistant, RAxisEquidistant> | |||
| hist(RAxisEquidistant(2, 0., 2.), RAxisEquidistant(2, -1., 1.)); | |||
|
|
|||
| // Here's a visual overview of how binning should work | |||
|
Starting build on |
…it) (root-project#4688) * Fix incorrect axis in RGetBinIndex axis growth logic * Fix off-by-one in RGetBinIndex need-to-grow detection logic * Make RFillBinCoord logic consistent with RGetBinIndex logic * Add tests of GetBinFrom/GetBinCenter/GetBinTo
…it) (root-project#4688) * Fix incorrect axis in RGetBinIndex axis growth logic * Fix off-by-one in RGetBinIndex need-to-grow detection logic * Make RFillBinCoord logic consistent with RGetBinIndex logic * Add tests of GetBinFrom/GetBinCenter/GetBinTo
Previously, ROOT7 histograms used different local bin coordinate conventions for GetBinIndex and GetBin(From|Center|To), as discussed in https://sft.its.cern.ch/jira/browse/ROOT-10445 . This PR brings order to that chaos, following the resolution proposed in https://sft.its.cern.ch/jira/browse/ROOT-10446 , and adds some tests which assert that the two binning conventions will remain in sync in the future.
While I was looking through the RHistImpl code, investigating further binning logic inconsistencies, I also spotted a few mistakes in the (currently unused) axis growth logic of GetBinIndex. So I corrected those along the way.