Repository navigation
[RF] Fix RooExtendPdf yield/shape in ranged extended fits - #22989
Merged
Merged
Conversation
RooExtendPdf is self-normalized, so it delegates both the shape normalization and the extended-term fraction to the wrapped pdf. In a fit over a sub-range, the fit machinery sets the normalization range only on the outer RooExtendPdf; it was never propagated to the wrapped pdf. As a result the wrapped pdf stayed normalized over the full range, which collapsed the fitted yield N to the number of events in the fit range (instead of reinterpreting it to the range requested in the constructor) and biased the shape parameters. This regressed with the multi-range fit rework in fa10523 (v6.28), and is visible in the rf204b_extendedLikelihood_rangedFit tutorial, where the RooExtendPdf fit disagreed with the equivalent RooAddPdf fit. Propagate the normalization range to the wrapped pdf during graph compilation, mirroring what RooAddPdf does for its components. This is gated on likelihood mode: for a binned chi2 the per-bin predictions must stay normalized over the full range so that the chi2 remains additive across sub-ranges. Closes root-project#22959. 🤖 Done with the help of AI.
The inline documentation of rf204b_extendedLikelihood_rangedFit had a frac = integral(Fit range) / integral(Full range) definition that is the reciprocal of what RooExtendPdf::expectedEvents() actually computes, and text saying the expected event count is divided by it. This went unnoticed because the ranged extended fit this tutorial demonstrates was broken since fa10523 and only fixed by the preceding commit, so the described numbers were never actually verified against real output. Also update the quoted RooAddPdf INFO log line, which was still from the pre-BatchMode RooAbsOptTestStatistic code path and no longer matches what fitTo() prints. Applies to both the C++ and Python versions of the tutorial.
guitargeek
changed the title
Test Results 23 files 23 suites 3d 19h 38m 45s ⏱️ Results for commit 1a753a8. |
dpiparo
approved these changes
Aug 4, 2026
dpiparo
left a comment
dpiparo
left a comment
Member
There was a problem hiding this comment.
Thanks for addressing this subtle issue.
Contributor
Author
|
/backport to 6.40 |
|
Preparing to backport PR #22989 to branch 6.40 requested by guitargeek |
|
This PR has been backported to branch 6.40: #23000 |
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.
RooExtendPdf is self-normalized, so it delegates both the shape normalization and the extended-term fraction to the wrapped pdf. In a fit over a sub-range, the fit machinery sets the normalization range only on the outer RooExtendPdf; it was never propagated to the wrapped pdf. As a result the wrapped pdf stayed normalized over the full range, which collapsed the fitted yield N to the number of events in the fit range (instead of reinterpreting it to the range requested in the constructor) and biased the shape parameters.
This regressed with the multi-range fit rework in https://github.com/root-project/root/commit fa10523 (v6.28), and is visible in the rf204b_extendedLikelihood_rangedFit tutorial, where the RooExtendPdf fit disagreed with the equivalent RooAddPdf fit.
Propagate the normalization range to the wrapped pdf during graph compilation, mirroring what RooAddPdf does for its components. This is gated on likelihood mode: for a binned chi2 the per-bin predictions must stay normalized over the full range so that the chi2 remains additive across sub-ranges.
Closes #22959.
🤖 Done with the help of AI.
A second commit fixes up minor issues with the comments in the related tutorial.
Note that this doesn't fix the old
"legacy"NLL evaluation backend, but this is on its way out anyway.