Skip to content

[RF] Fix implementation of RooGenericPdf/RooFormulaVar::binBoundaries()#22859

Merged
guitargeek merged 1 commit into
root-project:masterfrom
guitargeek:binning_followup
Jul 21, 2026
Merged

[RF] Fix implementation of RooGenericPdf/RooFormulaVar::binBoundaries()#22859
guitargeek merged 1 commit into
root-project:masterfrom
guitargeek:binning_followup

Conversation

@guitargeek

Copy link
Copy Markdown
Contributor

Follows up on 9ea4ec6, which got the implementation of RooFormulaVar::binBoundaries() and RooGenericPdf::binBoundaries() wrong. The limis passed to the binBoundaries() call should not be included in the returned std::list<double>.

I was led to believe that including the query boundaries was necessary, because otherwise the numeric integrals didn't work, but I see now that this is actually a bug on the RooBinIntegrator side, which is now reported as #22858.

Also, implement a unit test that ensures the fixed implementation of binBoundaries() is consistent with the one of RooHistPdf and matches the expected values.

Finally, move a remaining helper that was added in 9ea4ec6 from the public to the private interface (from RooHelpers.h to RooFitImplHelpers.h, which never gets installed).

Follows up on 9ea4ec6, which got the implementation of
`RooFormulaVar::binBoundaries()` and `RooGenericPdf::binBoundaries()`
wrong. The limis passed to the `binBoundaries()` call should not be
included in the returned `std::list<double>`.

I was led to believe that including the query boundaries was necessary,
because otherwise the numeric integrals didn't work, but I see now that
this is actually a bug on the RooBinIntegrator side, which is now
reported as root-project#22858.

Also, implement a unit test that ensures the fixed implementation of
`binBoundaries()` is consistent with the one of RooHistPdf and matches
the expected values.

Finally, move a remaining helper that was added in 9ea4ec6 from the
public to the private interface (from RooHelpers.h to
RooFitImplHelpers.h, which never gets installed).
@github-actions

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 14h 53m 33s ⏱️
 3 872 tests  3 871 ✅ 0 💤 1 ❌
78 773 runs  78 772 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 66519c4.

@dpiparo
dpiparo self-requested a review July 21, 2026 13:07
@guitargeek
guitargeek merged commit 28b3942 into root-project:master Jul 21, 2026
33 of 39 checks passed
@guitargeek
guitargeek deleted the binning_followup branch July 21, 2026 13:37
@guitargeek

Copy link
Copy Markdown
Contributor Author

/backport to 6.40

@root-project-bot

Copy link
Copy Markdown

Preparing to backport PR #22859 to branch 6.40 requested by guitargeek

@root-project-bot

Copy link
Copy Markdown

This PR has been backported to branch 6.40: #22868

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants