[RF] Fix RooProdPdf-wrapped RooAddPdf yield in ranged fits - #23017
Open
guitargeek wants to merge 1 commit into
Open
[RF] Fix RooProdPdf-wrapped RooAddPdf yield in ranged fits#23017guitargeek wants to merge 1 commit into
guitargeek wants to merge 1 commit into
Conversation
A RooProdPdf that wraps a RooAddPdf (the common way to attach constraint terms to a model, e.g. PROD::model(add, constraints)) gave a different fitted yield than the bare RooAddPdf in a ranged fit. The bare RooAddPdf reinterprets its coefficients with respect to the full range, but the one nested in the RooProdPdf did not, so its yield collapsed to the number of events inside the fit range instead. The reason is that the normalization range set on the RooProdPdf during a ranged fit was not propagated to its component pdfs, so the nested RooAddPdf never saw the fit range and skipped the coefficient reinterpretation. Propagate the normalization range to the components during compilation, mirroring what RooAddPdf already does for its own components. The range is only propagated to components for which it is actually defined. Constraint pdfs of nuisance parameters are normalized over observables that don't know about the fit range, and forcing it on them would be wrong (and for a multi-range even throws because the undefined sub-ranges collapse to the full range and overlap). Closes root-project#16673. 🤖 Done with the help of AI.
Test Results 23 files 23 suites 3d 21h 9m 54s ⏱️ For more details on these failures, see this check. Results for commit 87588ef. |
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
1 participant
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.
A RooProdPdf that wraps a RooAddPdf (the common way to attach constraint terms to a model, e.g. PROD::model(add, constraints)) gave a different fitted yield than the bare RooAddPdf in a ranged fit. The bare RooAddPdf reinterprets its coefficients with respect to the full range, but the one nested in the RooProdPdf did not, so its yield collapsed to the number of events inside the fit range instead.
The reason is that the normalization range set on the RooProdPdf during a ranged fit was not propagated to its component pdfs, so the nested RooAddPdf never saw the fit range and skipped the coefficient reinterpretation. Propagate the normalization range to the components during compilation, mirroring what RooAddPdf already does for its own components.
The range is only propagated to components for which it is actually defined. Constraint pdfs of nuisance parameters are normalized over observables that don't know about the fit range, and forcing it on them would be wrong (and for a multi-range even throws because the undefined sub-ranges collapse to the full range and overlap).
Closes #16673.
🤖 Done with the help of AI.