Skip to content

[RF] Let RooGenericPdf/RooFormulaVar declare a piecewise-flat binning#22708

Open
guitargeek wants to merge 1 commit into
root-project:masterfrom
guitargeek:flat_binning
Open

[RF] Let RooGenericPdf/RooFormulaVar declare a piecewise-flat binning#22708
guitargeek wants to merge 1 commit into
root-project:masterfrom
guitargeek:flat_binning

Conversation

@guitargeek

@guitargeek guitargeek commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

A RooGenericPdf or RooFormulaVar that is constant within bins of an observable (a step function) was always integrated with the generic adaptive numeric integrator, which is needlessly expensive: for a flat distribution the integral is just the sum of each bin's value times its width.

Add RooGenericPdf::setBinBoundaries() and RooFormulaVar::setBinBoundaries(), which take a RooAbsBinning for an observable and declare the function to be flat within those bins. Once set:

  • isBinnedDistribution() reports the observable as binned, so RooRealIntegral selects the fast RooBinIntegrator;
  • binBoundaries() and plotSamplingHint() expose the bins so that integration covers exactly the range and plotting draws crisp steps.

A binning can be registered for several observables (e.g. for a multi-dimensional flat distribution). By default the function is sampled inside each bin to verify that it really is flat, and the binning is rejected with an error otherwise; pass checkFlatness=false to skip this.

The binnings are stored in a std::map keyed by the observable's index in the internal variable list, so they survive renaming of a variable or a server redirection, and a RooUniformBinning keeps the storage compact even for many bins. Lookups resolve the observable by name, so a same-named stand-in (e.g. one read back separately from a file) is accepted too.

The shared flatness check and bin-boundary helpers live in RooHelpers. The persistent schema version of both classes is bumped to 2.

Feature requested by @Phmonski and @will-cern for supporting the ATLAS Higgs discovery workspaces without custom ATLAS RooFit classes.

@guitargeek guitargeek self-assigned this Jun 25, 2026
@guitargeek guitargeek requested a review from hageboeck as a code owner June 25, 2026 17:53
@guitargeek guitargeek added new feature in:RooFit experiment Affects an experiment / reported by its software & computimng experts labels Jun 25, 2026
A RooGenericPdf or RooFormulaVar that is constant within bins of an
observable (a step function) was always integrated with the generic
adaptive numeric integrator, which is needlessly expensive: for a flat
distribution the integral is just the sum of each bin's value times its
width.

Add RooGenericPdf::setBinBoundaries() and RooFormulaVar::setBinBoundaries(),
which take a RooAbsBinning for an observable and declare the function to be
flat within those bins. Once set:

  * isBinnedDistribution() reports the observable as binned, so
    RooRealIntegral selects the fast RooBinIntegrator;
  * binBoundaries() and plotSamplingHint() expose the bins so that
    integration covers exactly the range and plotting draws crisp steps.

A binning can be registered for several observables (e.g. for a
multi-dimensional flat distribution). By default the function is sampled
inside each bin to verify that it really is flat, and the binning is
rejected with an error otherwise; pass checkFlatness=false to skip this.

The binnings are stored in a std::map keyed by the observable's index in
the internal variable list, so they survive renaming of a variable or a
server redirection, and a RooUniformBinning keeps the storage compact even
for many bins. Lookups resolve the observable by name, so a same-named
stand-in (e.g. one read back separately from a file) is accepted too.

The shared flatness check and bin-boundary helpers live in RooHelpers.
The persistent schema version of both classes is bumped to 2.
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 15h 14m 19s ⏱️
 3 875 tests  3 869 ✅   0 💤 6 ❌
78 804 runs  78 691 ✅ 107 💤 6 ❌

For more details on these failures, see this check.

Results for commit 8d9620e.

♻️ This comment has been updated with latest results.

}
for (RooAbsArg *o : obs) {
if (_binnings.find(_actualVars.index(o->GetName())) == _binnings.end()) {
return false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if obs contains a variable that we do not depend on, will this return false then? If I look at behaviour of e.g. RooProduct's implementation: https://root.cern.ch/doc/master/RooProduct_8cxx_source.html#l00420 wont it return true if it doesn't depend on the variables passed?

const double value = integrate(pdf, x, integratorName);
EXPECT_NE(integratorName.find("RooBinIntegrator"), std::string::npos) << integratorName;
EXPECT_DOUBLE_EQ(value, piecewiseFlatIntegral);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps nice to have a test to show that non-uniform binning is also integrated correctly (I hope?)

// bin integrator instead of the generic numeric integrator. A binning can be
// set for more than one observable. Use a RooUniformBinning to describe many
// uniform bins compactly.
void setBinBoundaries(RooAbsRealLValue &obs, const RooAbsBinning &binning, bool checkFlatness = true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of me thinks it would be nice that user can reverse this operation, but this method doesn't allow for that? If don't want to change the signature, then have a removeBinBoundaries(RooAbsRealLValue &obs) method or similar?

Also why RooAbsRealLValue &obs and not const RooAbsRealLValue &obs ?

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

Labels

🤖AI-Assisted experiment Affects an experiment / reported by its software & computimng experts in:RooFit new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants