Repository navigation
[RF] Keep RooRealSumPdf integral code out of the codegen event loop - #23723
Merged
guitargeek merged 1 commit intoOct 15, 2026
Merged
guitargeek merged 1 commit into
guitargeek merged 1 commit into
Conversation
The integral of a RooRealSumPdf reuses the translation of the sum
itself, which placed its generated code via addToCodeBody(&arg, ...).
Since the sum depends on the observables, the integral code ended up
inside the event loop, even though the integral itself is
data-independent.
For an extended RooRealSumPdf, the same integral is the pdf
normalization inside the loop and the expected number of events that the
extended term reads after the loop. Because codegen caches results by
node name, the post-loop term referenced a loop-local variable and cling
refused to compile the generated function:
error: use of undeclared identifier 't14'
RooNLLVarNewResult += t14 - RooNLLVarNewWeightSum * std::log(t14);
Decide the scope of the generated code from the inputs of the sum
instead of from the pdf node: the code is scope-independent if all
functions (or component integrals) and coefficients are. For the pdf
itself this is equivalent to the previous check, because the sum depends
on the data exactly when one of its inputs does. For the integral it
keeps the code outside the loop unless a coefficient depends on the
data. This needs CodegenContext::isScopeIndependent() to be public.
Add an extended RooRealSumPdf case to testRooFuncWrapper that fails
without the fix.
🤖 Done with the help of AI
Test Results 24 files 24 suites 4d 3h 5m 29s ⏱️ For more details on these failures, see this check. Results for commit 6d079f2. ♻️ This comment has been updated with latest results. |
dpiparo
approved these changes
Oct 15, 2026
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
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.
The integral of a RooRealSumPdf reuses the translation of the sum itself, which placed its generated code via addToCodeBody(&arg, ...). Since the sum depends on the observables, the integral code ended up inside the event loop, even though the integral itself is data-independent.
For an extended RooRealSumPdf, the same integral is the pdf normalization inside the loop and the expected number of events that the extended term reads after the loop. Because codegen caches results by node name, the post-loop term referenced a loop-local variable and cling refused to compile the generated function:
Decide the scope of the generated code from the inputs of the sum instead of from the pdf node: the code is scope-independent if all functions (or component integrals) and coefficients are. For the pdf itself this is equivalent to the previous check, because the sum depends on the data exactly when one of its inputs does. For the integral it keeps the code outside the loop unless a coefficient depends on the data. This needs CodegenContext::isScopeIndependent() to be public.
Add an extended RooRealSumPdf case to testRooFuncWrapper that fails without the fix.
🤖 Done with the help of AI