Skip to content

Commit 36f6400

Browse files
committed
[RF] Fix codegen of RooMultiVarGaussian
The codegen implementation for this class was broken because the function only took one template parameter, which resulted in overload resolution failure when the passed argument types were different. This is fixed now, and a unit test is added to prevent future regressions. Also, a workaround around a Clad issue is removed from the tests, and the tolerance parameter of some tests is adjusted, because changing a prior test had an effect on the random number generation of the Landau tests.
1 parent 33f0cd1 commit 36f6400

File tree

2 files changed

+39
-13
lines changed

2 files changed

+39
-13
lines changed

roofit/roofitcore/inc/RooFit/Detail/MathFuncs.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -787,8 +787,8 @@ double bernsteinIntegral(double xlo, double xhi, double xmin, double xmax, Doubl
787787
return norm * (xmax - xmin);
788788
}
789789

790-
template <typename DoubleArray>
791-
double multiVarGaussian(int n, DoubleArray x, DoubleArray mu, DoubleArray covI)
790+
template <typename XArray, typename MuArray, typename CovArray>
791+
double multiVarGaussian(int n, XArray x, MuArray mu, CovArray covI)
792792
{
793793
double result = 0.0;
794794

roofit/roofitcore/test/testRooFuncWrapper.cxx

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <RooHistFunc.h>
2626
#include <RooHistPdf.h>
2727
#include <RooMinimizer.h>
28+
#include <RooMultiVarGaussian.h>
2829
#include <RooPoisson.h>
2930
#include <RooPolynomial.h>
3031
#include <RooRealSumPdf.h>
@@ -448,12 +449,8 @@ FactoryTestParams param11{"ClassFactory1D",
448449
RooRealVar mu{"mu", "mu", 5, 0, 10};
449450
RooRealVar sigma{"sigma", "sigma", 2.0, 0.1, 10};
450451

451-
// TODO: When Clad issue #635 is solved, we can
452-
// actually use a complete Gaussian here, also
453-
// with sigma.
454452
std::unique_ptr<RooAbsPdf> pdf{RooClassFactory::makePdfInstance(
455-
//"model", "std::exp(-0.5 * (x - mu)*(x - mu) / (sigma * sigma))", {x, mu, sigma})};
456-
"model", "std::exp(-0.5 * (x - mu)*(x - mu))", {x, mu})};
453+
"model", "std::exp(-0.5 * (x - mu)*(x - mu) / (sigma * sigma))", {x, mu, sigma})};
457454
ws.import(*pdf);
458455
ws.defineSet("observables", "x");
459456
},
@@ -463,6 +460,34 @@ FactoryTestParams param11{"ClassFactory1D",
463460
5e-3, // increase tolerance because the numeric integration algos are still different
464461
/*randomizeParameters=*/true};
465462

463+
FactoryTestParams param12{"RooMultiVarGaussian",
464+
[](RooWorkspace &ws) {
465+
RooRealVar x("x", "x variable", -5, 5);
466+
RooRealVar y("y", "y variable", -5, 5);
467+
468+
RooArgList vars(x, y);
469+
470+
RooRealVar mean_x("mean_x", "mean of x", 1.0, -5, 5);
471+
RooRealVar mean_y("mean_y", "mean of y", -1.0, -5, 5);
472+
RooArgList means(mean_x, mean_y);
473+
474+
TMatrixDSym cov(2);
475+
cov(0, 0) = 1.0; // Var(x)
476+
cov(1, 1) = 1.5; // Var(y)
477+
cov(0, 1) = 0.3; // Cov(x,y)
478+
cov(1, 0) = 0.3;
479+
480+
RooMultiVarGaussian mvgauss("model", "Multivariate Gaussian", vars, means, cov);
481+
482+
ws.import(mvgauss);
483+
ws.defineSet("observables", vars);
484+
},
485+
[](RooAbsPdf &pdf, RooAbsData &data, RooWorkspace &, RooFit::EvalBackend backend) {
486+
return std::unique_ptr<RooAbsReal>{pdf.createNLL(data, backend)};
487+
},
488+
5e-3, // increase tolerance because the numeric integration algos are still different
489+
/*randomizeParameters=*/true};
490+
466491
FactoryTestParams makeTestParams(const char *name, std::vector<std::string> const &expressions,
467492
double fitResultTolerance, bool randomizeParameters = true)
468493
{
@@ -515,11 +540,11 @@ auto testValues = testing::Values(
515540
// We're testing several Landau configurations, because the underlying
516541
// ROOT::Math::landau_cdf is defined piecewise. Like this, we're covering
517542
// all possible code paths in the pullback.
518-
makeTestParams("RooLandau1", {"Landau::model(x[5., 0., 30.], ml[6., 1., 30.], sl[1., 0.01, 50.])"}, 6e-3, false),
519-
makeTestParams("RooLandau2", {"Landau::model(x[5., 0., 30.], ml[6., 1., 30.], sl[2.1, 0.01, 50.])"}, 6e-3, false),
520-
makeTestParams("RooLandau3", {"Landau::model(x[5., 0., 30.], ml[6., 1., 30.], sl[10., 0.01, 50.])"}, 6e-3, false),
521-
makeTestParams("RooLandau4", {"Landau::model(x[5., 0., 30.], ml[6., 1., 30.], sl[0.3, 0.01, 50.])"}, 6e-3, false),
522-
makeTestParams("RooLandau5", {"Landau::model(x[5., 0., 30.], ml[6., 1., 30.], sl[0.07, 0.01, 50.])"}, 6e-3, false),
543+
makeTestParams("RooLandau1", {"Landau::model(x[5., 0., 30.], ml[6., 1., 30.], sl[1., 0.01, 50.])"}, 7e-3, false),
544+
makeTestParams("RooLandau2", {"Landau::model(x[5., 0., 30.], ml[6., 1., 30.], sl[2.1, 0.01, 50.])"}, 7e-3, false),
545+
makeTestParams("RooLandau3", {"Landau::model(x[5., 0., 30.], ml[6., 1., 30.], sl[10., 0.01, 50.])"}, 7e-3, false),
546+
makeTestParams("RooLandau4", {"Landau::model(x[5., 0., 30.], ml[6., 1., 30.], sl[0.3, 0.01, 50.])"}, 7e-3, false),
547+
makeTestParams("RooLandau5", {"Landau::model(x[5., 0., 30.], ml[6., 1., 30.], sl[0.07, 0.01, 50.])"}, 7e-3, false),
523548
makeTestParams(
524549
"RooRealSumPdf1",
525550
{"Gaussian::gx(x[-10,10],m[0],1.0)", "Chebychev::ch(x,{0.1,0.2,-0.3})", "RealSumPdf::model({gx, ch}, {f[0,1]})"},
@@ -530,7 +555,8 @@ auto testValues = testing::Values(
530555
{"x[-10., 10.]", "mean[1., -10., 10.]", "sigma[1., 0.1, 10.]",
531556
"expr::gauss_func('std::exp(-0.5*(x - mean) * (x - mean) / (sigma * sigma))', {x, mean, sigma})",
532557
"WrapperPdf::model(gauss_func)"},
533-
6e-3, true));
558+
6e-3, true),
559+
param12);
534560

535561
INSTANTIATE_TEST_SUITE_P(RooFuncWrapper, FactoryTest, testValues,
536562
[](testing::TestParamInfo<FactoryTest::ParamType> const &paramInfo) {

0 commit comments

Comments
 (0)