[RF] Correctly sync proxy normalization sets in RooAddPdf::getValV()#10885
[RF] Correctly sync proxy normalization sets in RooAddPdf::getValV()#10885guitargeek merged 1 commit intoroot-project:masterfrom
RooAddPdf::getValV()#10885Conversation
When getValV() was directly implemented in RooAddPdf, it was missed to copy-paste the part from RooAbsPdf::getValV() where the normalization sets for the proxies was synced. A unit test with the reproducer for an issue caused by missing the syncing is also introduced with this commit, involving the SPlot from RooStats. Fixes root-project#10869.
|
Starting build on |
|
Build failed on ROOT-performance-centos8-multicore/cxx17. Failing tests: |
lmoneta
left a comment
lmoneta
left a comment
There was a problem hiding this comment.
LGTM!
Just one comment that I am not sure how relevant it is.
Thank you for the fix and nice you add also a test
| } | ||
| // Do running sum of coef/pdf pairs, calculate lastCoef. | ||
| if (isValueDirty() || nsetChanged) { | ||
| _value = 0.0; |
There was a problem hiding this comment.
Why are you using here _value instead of a local value variable ?
Is this intended ?
There was a problem hiding this comment.
Yes this it intended (it's the same code as in RooAbsPdf). We want to calculate in-place the _value variable such that the value can be retrieved directly from there then the AddPdf is not dirty when evaluated the next time.
3 participants
When getValV() was directly implemented in RooAddPdf, it was missed to
copy-paste the part from RooAbsPdf::getValV() where the normalization
sets for the proxies was synced.
A unit test with the reproducer for an issue caused by missing the
syncing is also introduced with this commit, involving the SPlot from
RooStats.
Closes #10869.