Skip to content

Commit

Permalink
[RF] Fixup to parameter index calculation in ParamHistFunc
Browse files Browse the repository at this point in the history
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
  • Loading branch information
guitargeek committed Jul 19, 2022
1 parent 2e38273 commit 8cda6aa
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions roofit/histfactory/src/ParamHistFunc.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,13 @@ RooAbsReal& ParamHistFunc::getParameter( Int_t index ) const {
_numBinsPerDim = getNumBinsPerDim(_dataVars);
}

int i = index / n.yz ;
int j = (index % n.y) / n.z;
int k = index % (n.yz);
// Unravel the index to 3D coordinates. We can't use the index directly,
// 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 % n.yz;
const int j = tmp / n.z;
const int k = tmp % n.z;

return static_cast<RooAbsReal&>(_paramSet[i + j * n.x + k * n.xy]);
}
Expand Down

0 comments on commit 8cda6aa

Please sign in to comment.