[RF] Avoid RooAbsPdf::expectedEvents() calls in RooAddPdf::doEval()#21620
Merged
guitargeek merged 3 commits intoroot-project:masterfrom Mar 17, 2026
Merged
[RF] Avoid RooAbsPdf::expectedEvents() calls in RooAddPdf::doEval()#21620guitargeek merged 3 commits intoroot-project:masterfrom
RooAbsPdf::expectedEvents() calls in RooAddPdf::doEval()#21620guitargeek merged 3 commits intoroot-project:masterfrom
Conversation
Rule of five: if the copy constructor and destructor are implemented, one also has to implement copy assignment, and the move constructor and operator. The copy assignment here should mimic the copy constructor, and the move assignment and constructor can be the default (although it's good practive to make this explicit the make clear the intent).
This avoids name collisions of objects that are semantically different but have the same name.
When evaluating pdfs with `RooAbsReal::doEval()` in the new RooFit evaluation backend, we have to avoid calling back into the old code paths that use `getVal()`. One place where this still happened was the RooAbsPdf, under the condition that that the coefficients represent expected numbers of events. It would call `RooAbsPdf::expectedEvents()` when updating the coefficients, which uses `getVal()`. Instead, the coefficients need to be replaced with RooAbsReals that represent the expected numbers of events, to fully expose this in the computation graph. Like this, we don't have to rely on the legacy "dirty state propagation" that is important for the `getVal()` code path.
Test Results 22 files 22 suites 3d 2h 12m 10s ⏱️ Results for commit 3419e65. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When evaluating pdfs with
RooAbsReal::doEval()in the new RooFit evaluation backend, we have to avoid calling back into the old code paths that usegetVal(). One place where this still happened was the RooAbsPdf, under the condition that that the coefficients represent expected numbers of events. It would callRooAbsPdf::expectedEvents()when updating the coefficients, which usesgetVal().Instead, the coefficients need to be replaced with RooAbsReals that represent the expected numbers of events, to fully expose this in the computation graph. Like this, we don't have to rely on the legacy "dirty state propagation" that is important for the
getVal()code path.A second commit in this PR avoids some name collisions of RooFit object in the new extended computation graph, and a third commit is about implementing the copy assignment for the
RooAICRegistryclass correctly, which is now hit.