[6.40] [RF] Fix numerical precision of RooIntegralMorph - #23264
Open
root-project-bot wants to merge 3 commits into
Open
[6.40] [RF] Fix numerical precision of RooIntegralMorph#23264root-project-bot wants to merge 3 commits into
root-project-bot wants to merge 3 commits into
Conversation
The precision of RooIntegralMorph ("integral morphing", Alex Read's
c.d.f. interpolation) was reported in Baak et al., NIM A 771 (2015)
39-48, Table 2, to be limited to Kolmogorov-Smirnov distances of about
1e-3 from the exact morphed shape, even for pure Gaussian inputs where
the method is mathematically exact. The method itself supports precision
at the 1e-14 level, as in Read's original Fortran implementation, so
this pointed to implementation defects. Three are fixed here:
1. Unit-mixing bug in MorphCacheElem::interpolateGap(): the x offset of
the gap boundary from its bin center was added to a bin *count* and
multiplied by a per-bin y increment without dividing by the bin
width first. This mis-assigned interpolated c.d.f. values by up to
half a bin, causing percent-level errors in the cached p.d.f. bin
values (the dominant defect).
2. The y values produced by the recursive gap-filling heuristics (with
their 1% "centration quality" tolerance) were used as-is. They are
now polished per bin with Newton iterations on the monotone map
X(y) = alpha*x1(y) + (1-alpha)*x2(y), using
dX/dy = alpha/f1(x1) + (1-alpha)/f2(x2), so every cached bin value
corresponds to its bin center to root-finder precision. This also
fixes an off-by-one in the transfer loop that zeroed the highest
calculable bin and left the last histogram bin unfilled.
3. The tail cutoff _ycutoff is lowered from 1e-7 to 1e-12, which
removes the corresponding 3e-7 floor on the K-S distance from
truncating the p.d.f. tails.
With these fixes the cached shape is accurate up to the resolution of
the cache histogram itself: the K-S distance for the Gaussian benchmark
of the paper drops from 3e-4 to 1.3e-7 at 1000 cache bins and reaches
5e-11 at ~30000 bins, matching an ideal same-binned representation of
the exact result at every binning tested, at a cache-fill cost of a few
tens of milliseconds.
Also error out with a clear message instead of silently producing an
all-zero p.d.f. when the cache binning has no bins (the default since
variables default to zero bins).
Closes https://its.cern.ch/jira/browse/ROOT-10453
🤖 Done with the help of AI
(cherry picked from commit 6815e26)
The numerical precision fix for RooIntegralMorph in the previous commit changes the pdf values at the 1e-3 to 1e-2 level, so the accept-reject toy generation in stressRooFit test 705 now draws a slightly different dataset than the one frozen in stressRooFit_ref.root: the comparison of the toy data histogram h_lmorphData in rf705_plot2 fails with a Kolmogorov distance of M = 0.005 against a tolerance of 5e-4, while the corresponding Kolmogorov probability is 1.0, i.e. the new sample is fully compatible statistically. All pdf shape comparisons still pass even against the old reference. Instead of regenerating the binary reference file, the test is replaced by unit tests in testRooIntegralMorph.cxx with the same coverage that need no stored reference results: - GaussPolyMorphAccuracy morphs the same Gaussian and polynomial shapes as the old test and compares against the exact morphed shape, which is computed in the test itself from the analytic c.d.f.s of the endpoint shapes (the morphed c.d.f. is known parametrically through the quantile functions). This checks the actual pdf values about two orders of magnitude more tightly than the 5e-2 curve tolerance of the old stress test, while being insensitive to expected changes in generated toy samples. - AlphaCacheScan covers the two-dimensional (x, alpha) cache with setCacheAlpha(true), scanning alpha values between the cache bin centers against the same exact reference. - GenerateAndFit covers toy generation from the morph pdf and the fit back with the alpha cache enabled, on both the legacy and cpu evaluation backends (the backends that previously exercised test 705; it was skipped for the codegen backends). The now-unused rf705_* keys remain in stressRooFit_ref.root to avoid binary file churn; they are harmless dead entries. 🤖 Done with the help of AI (cherry picked from commit 1962080)
(cherry picked from commit 34390cb)
root-project-bot
requested review from
bellenot and
guitargeek
as code owners
September 5, 2026 10:08
Test Results 22 files 22 suites 3d 11h 31m 27s ⏱️ For more details on these failures, see this check. Results for commit 1dc1fbf. |
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
3 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.
Backport of #23181, requested by @guitargeek.