[RF] Don't deep clone the RooAbsArg in RooAbsReal::getPropagatedError
#10762
Conversation
|
Starting build on |
|
Build failed on ROOT-ubuntu16/nortcxxmod. Errors:
|
guitargeek
force-pushed
the
getPropagatedError_1
branch
from
21879fc to
d78f5b3
Compare
|
Starting build on |
|
Build failed on ROOT-ubuntu16/nortcxxmod. Errors:
|
|
Build failed on ROOT-performance-centos8-multicore/cxx17. Failing tests: |
The `RooAbsReal::getPropagatedError()` function was using some of the most expensive operations in RooFit for larger computation graphs: cloning the model, and figuring out parameters and observables. This was done for no apparent reason, as the `RooAbsReal` is not mutated by `getPropagaterError`. Parameter values are slightly changed for reevaluation, but they are reset right after. A final call to `getVal()` is enough to reset the original state, which is much more efficient than cloning everything. This commit also adds a check if the parameters in the RooAbsReal have the same values as in the fit result (otherwise the logic of getPropagatedError() is broken). This change was motivated by the following forum post: https://root-forum.cern.ch/t/getpropagatederror-method-taking-too-long-to-run/50392
guitargeek
force-pushed
the
getPropagatedError_1
branch
from
d78f5b3 to
7979d21
Compare
|
Starting build on |
|
Build failed on mac1015/cxx17. Failing tests: |
The
RooAbsReal::getPropagatedError()function was using some of themost expensive operations in RooFit for larger computation graphs:
cloning the model, and figuring out parameters and observables.
This was done for no apparent reason, as the
RooAbsRealis not mutatedby
getPropagaterError. Parameter values are slightly changed forreevaluation, but they are reset right after. A final call to
getVal()is enough to reset the original state, which is much more efficient than
cloning everything.
This change was motivated by the following forum post:
https://root-forum.cern.ch/t/getpropagatederror-method-taking-too-long-to-run/50392