Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RF] RooAddPdf::fixCoefRange cache issue with createIntegral #10988

Closed
tcuisset opened this issue Jul 19, 2022 · 1 comment · Fixed by #10995
Closed

[RF] RooAddPdf::fixCoefRange cache issue with createIntegral #10988

tcuisset opened this issue Jul 19, 2022 · 1 comment · Fixed by #10995

Comments

@tcuisset
Copy link

Describe the bug

I am experiencing strange behaviour with RooAddPdf::fixCoefRange. When I call createIntegral on a RooAddPdf, then call fixCoefRange, then call createIntegral again, the same value is returned (whilst the change of definition of the coefficients should change the integral). It seems to me the cache does not get cleared when calling fixCoefRange, as the behaviour is correct when calling fixCoefRange before any integral computation.

To Reproduce

RooWorkspace w1;
w1.factory("x[3., 0., 10.]");
w1.var("x")->setRange("range_int", 0., 4.);
w1.factory("AddPdf::sum(Gaussian(x, mean1[1.], sigma1[2.]), Gaussian(x, mean2[5.], sigma2[10.]), coef[0.3])");
RooWorkspace w2(w1);

//Call createIntegral on workspace w1 only
cout << w1.pdf("sum")->createIntegral(RooArgSet(*w1.var("x")),RooFit::NormSet(RooArgSet(*w1.var("x"))), RooFit::Range("range_int"))->getVal() << endl;

w1.var("x")->setRange("fixCoefRange", 0.,  1.);
static_cast<RooAddPdf*>(w1.pdf("sum"))->fixCoefRange("fixCoefRange");

w2.var("x")->setRange("fixCoefRange", 0.,  1.);
static_cast<RooAddPdf*>(w2.pdf("sum"))->fixCoefRange("fixCoefRange");

cout << w1.pdf("sum")->createIntegral(RooArgSet(*w1.var("x")),RooFit::NormSet(RooArgSet(*w1.var("x"))), RooFit::Range("range_int"))->getVal() << endl;
cout << w2.pdf("sum")->createIntegral(RooArgSet(*w2.var("x")),RooFit::NormSet(RooArgSet(*w2.var("x"))), RooFit::Range("range_int"))->getVal() << endl;

This prints :

0.548209 //Before calling RooAddPdf::fixCoefRange
0.548209 //After calling RooAddPdf::fixCoefRange but with a previous call to createIntegral
0.463998 //After calling RooAddPdf::fixCoefRange without any previous call

whilst I would expect :

0.548209
0.463998
0.463998

Setup

ROOT 6.26/02

@tcuisset tcuisset added the bug label Jul 19, 2022
@guitargeek guitargeek self-assigned this Jul 19, 2022
@guitargeek guitargeek changed the title RooAddPdf::fixCoefRange cache issue with createIntegral [RF] RooAddPdf::fixCoefRange cache issue with createIntegral Jul 19, 2022
guitargeek added a commit to guitargeek/root that referenced this issue Jul 19, 2022
When the normalization range for coefficient determination of a
RooAddPdf is changed, the AddPdf's projection cache needs to be reset,
just like it is already done in `RooAddPdf::fixCoefNormalization`.
Otherwise, there will be problems in the pdf evaluation and integration
because the projection cache is invalid.

A unit test based on the GitHub issue that reported this problem is also
implemented.

Closes root-project#10988.
@guitargeek
Copy link
Contributor

Thanks for reporting the problem! I have opened a PR to fix it: #10995

guitargeek added a commit that referenced this issue Jul 20, 2022
When the normalization range for coefficient determination of a
RooAddPdf is changed, the AddPdf's projection cache needs to be reset,
just like it is already done in `RooAddPdf::fixCoefNormalization`.
Otherwise, there will be problems in the pdf evaluation and integration
because the projection cache is invalid.

A unit test based on the GitHub issue that reported this problem is also
implemented.

Closes #10988.
j-mathe pushed a commit to j-mathe/root that referenced this issue Jul 26, 2022
When the normalization range for coefficient determination of a
RooAddPdf is changed, the AddPdf's projection cache needs to be reset,
just like it is already done in `RooAddPdf::fixCoefNormalization`.
Otherwise, there will be problems in the pdf evaluation and integration
because the projection cache is invalid.

A unit test based on the GitHub issue that reported this problem is also
implemented.

Closes root-project#10988.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants