[RF] Fixup to parameter index calculation in ParamHistFunc #10989
Conversation
|
Starting build on |
| // because in the parameter set the dimensions are ordered in reverse order | ||
| // compared to the RooDataHist (for historical reasons). | ||
| const int i = index / n.yz; | ||
| const int tmp = index - i * n.yz; |
There was a problem hiding this comment.
It is not equivalent to
temp=index%n.yz?
There was a problem hiding this comment.
Thanks, you are right! The expression for tmp and k could be simplified, which I changed now.
Last year, with commit 3657e7c, the parameter index calculation was changed to be on the fly instead of using a look-up map, which is much faster. However, the implemented formula was not correct for two or three dimensions, which is fixed by this commit. To make sure that the index computation is correct this time, the new code was tested in this code snippet with various inputs: ```C++ void runTest(int nx = 42, int ny = 42, int nz = 42) { const int nxy = nx * ny; const int nyz = ny * nz; for (int i = 0; i < nx; ++i) { for (int j = 0; j < ny; ++j) { for (int k = 0; k < nz; ++k) { const int index = k + j * nz + i * ny * nz; const int gammaIndex = i + j * nx + k * nx * ny; const int i2 = index / nyz; const int tmp = index % nyz; const int j2 = tmp / nz; const int k2 = tmp % nz; const int gammaIndex2 = i2 + j2 * nx + k2 * nxy; if (gammaIndex2 != gammaIndex) { std::cout << "The unraveled indices were not correct!" << std::endl; return; } } } } } ``` Needs to be backported to the 6.26 branch to get into the 6.26.06 patch release. This commit the following problem reported on the forum: https://root-forum.cern.ch/t/cpycppyy-segfault-on-mac-m1/50822
guitargeek
force-pushed
the
ParamHistFunc_fixup
branch
from
13b3e3f to
4b05e75
Compare
|
Starting build on |
|
Build failed on ROOT-performance-centos8-multicore/cxx17. Failing tests: |
|
Build failed on mac1015/cxx17. Failing tests: |
Last year, with commit 3657e7c, the parameter index calculation was
changed to be on the fly instead of using a look-up map, which is much
faster.
However, the implemented formula was not correct for two or three
dimensions, which is fixed by this commit.
To make sure that the index computation is correct this time, the new
code was tested in this code snippet with various inputs:
Needs to be backported to the 6.26 branch to get into the 6.26.06 patch
release.
This commit the following problem reported on the forum:
https://root-forum.cern.ch/t/cpycppyy-segfault-on-mac-m1/50822