Explain what you would like to see improved and how.
Related PRs: #23298, #15543, #11171, #12955
Motivation
While working on #23298, the question came up whether the computation of a histogram bin index should be represented by a standalone RooFit object, tentatively called RooBinIndex.
The immediate motivation is RooBinWidthFunction: after #23298 it depends directly on its observables, but consequently needs to determine the current multi-dimensional bin itself in both scalar and batch evaluation. This logic is not specific to bin widths, however. Similar coordinate-to-bin-index computations already exist in several RooFit and HistFactory classes.
A first-class RooBinIndex could provide a single graph node representing
observables -> flattened bin index
and allow multiple histogram-like objects with the same binning to share that result.
This issue is intended to collect the relevant use cases and design questions. It could either serve as the starting point for a refactor now, or remain as follow-up work after merging #23298 independently.
Existing implementations / potential consumers
The following classes currently contain or rely on closely related bin-index logic:
RooBinWidthFunction
The implementation in #23298 computes the current bin directly from its observable list, including multi-dimensional index flattening and separate scalar/batch implementations.
This is the immediate use case that prompted the discussion.
RooHistFunc
RooHistFunc currently has
Int_t getBin() const;
std::vector<Int_t> getBins(RooFit::EvalContext &ctx) const;
which effectively implement a private version of the proposed abstraction.
Its evaluation code additionally has separate scalar, 1D batch, and ND batch paths for mapping function observables onto the underlying histogram bins.
RooHistPdf
RooHistPdf performs essentially the same mapping from PDF observables to the bins of its RooDataHist.
For zero-order histogram evaluation this could naturally become
RooBinIndex -> weight[index]
Interpolation would still require the actual coordinates and neighbouring bin geometry, so RooBinIndex would only represent the central-bin lookup.
RooParamHistFunc
This is perhaps the simplest potential consumer. Its scalar evaluation currently amounts to
idx = dataHist.getIndex(observables);
return parameters[idx];
while code generation independently asks RooDataHist to generate the corresponding tree-index expression.
This could naturally become a parameter lookup driven by a RooBinIndex.
HistFactory ParamHistFunc
The HistFactory implementation contains several versions of the same operation:
- scalar evaluation through
RooDataHist::getIndex();
- vectorized evaluation using
RooAbsBinning::binNumbers() and explicit index multipliers;
- code generation through
RooDataHist::calculateTreeIndexForCodeSquash().
This is a strong candidate for consolidation.
PiecewiseInterpolation
#15543 already introduced an optimization based on the fact that the nominal and variation RooHistFuncs in HistFactory use the same bin structure.
The generated code computes the bin index only once and then uses it to address the nominal, low, and high arrays.
A first-class RooBinIndex could make this sharing explicit in the RooFit computation graph instead of rediscovering it in code generation.
Conceptually, a HistFactory channel could then look like
+-> nominal histogram
+-> systematic variations
observables -> BinIndex -+-> MC-stat parameters
+-> bin-width correction
+-> other bin-wise quantities
RooDataHist
RooDataHist::calcTreeIndex() is currently the main general implementation of multi-dimensional bin-index calculation, and calculateTreeIndexForCodeSquash() implements the corresponding generated expression.
It probably should share an implementation with RooBinIndex, but RooDataHist itself should not necessarily depend on a graph node: it also needs to calculate indices for arbitrary coordinate collections outside a computation graph.
A possible layering would therefore be
common bin-index implementation
/ \
RooBinIndex RooDataHist
graph node storage API
Secondary candidates
RooBinSamplingPdf also has duplicated scalar/batch bin lookup: scalar evaluation uses getBin(), while batch evaluation independently determines the bin from the boundaries.
RooBinningCategory is conceptually very close to a one-dimensional RooBinIndex: it maps a real l-value through a named binning onto a category whose index corresponds to the bin number. It probably should not disappear, since the category labels are useful, but the two implementations might eventually share their underlying bin-lookup machinery.
Other uses of binNumber() such as in RooFFTConvPdf, RooPlot, or RooMomentMorphFuncND appear to be local geometry/cache algorithms rather than reusable graph-level bin-index computations, so they probably do not need to participate in this refactor.
Design questions
A reusable RooBinIndex turns out to require a few architectural decisions.
1. Which binning defines the index?
For RooBinWidthFunction after #23298, the live observables and their current default binnings are authoritative.
For RooHistFunc and RooHistPdf, however, the values can come from one set of observables while the relevant bin geometry comes from the corresponding histogram observables / RooDataHist.
Therefore an API that only stores
RooBinIndex(name, observables);
and always uses observables[i].getBinning() may be too restrictive.
The value source and the binning source may need to be representable separately.
2. What is the multi-dimensional flattening convention?
RooDataHist and HistFactory do not universally use the same ordering.
RooDataHist indexing makes the last listed dimension the fastest-running one.
The legacy HistFactory ParamHistFunc, on the other hand, stores parameters in TH1 ordering, where the first/x dimension is fastest, and contains explicit conversion logic between the two conventions.
A standalone RooBinIndex should have a clearly defined canonical ordering. Consumers with legacy storage conventions could then transform their arrays explicitly if necessary.
3. How are categories handled?
For categories, the histogram-bin number is the ordinal state number, which is not necessarily identical to the category's numeric state index.
Scalar evaluation can use RooAbsCategoryLValue::getBin(), but batch evaluation sees the numeric category values and therefore needs a proper value-to-ordinal mapping.
This needs to be handled centrally if RooBinIndex is intended to support mixed real/category histograms.
4. Dynamic and named binnings
The implementation should define how it behaves when
- a binning is replaced;
- the range or number of bins changes;
- a named rather than default binning is used;
- a parameterized/transformed binning changes dynamically.
This is already relevant in #23298 because RooBinWidthFunction needs to detect binning changes that do not necessarily propagate through the usual dirty-state mechanism.
5. Evaluation backends
If introduced as a RooFit graph node, RooBinIndex should ideally have consistent support for
- scalar evaluation;
EvalContext batch evaluation;
- code generation / code squashing.
Otherwise it risks simply moving the current duplication to a different layer.
Possible scope
There seem to be two reasonable levels of implementation.
Minimal extraction
Introduce RooBinIndex primarily for #23298 and the immediately duplicated helpers:
- new
RooBinIndex;
- use it in
RooBinWidthFunction;
- remove or replace
RooHistFunc::getBin() / getBins() where appropriate.
This would be a relatively small follow-up and would not require restructuring histogram evaluation more generally.
Canonical bin-index node
Make RooBinIndex the common graph-level representation of coordinate-to-bin mapping and adapt at least:
RooBinWidthFunction;
RooHistFunc;
RooHistPdf;
RooParamHistFunc;
- HistFactory
ParamHistFunc;
PiecewiseInterpolation;
- the relevant
RooDataHist helper implementation.
A more exhaustive cleanup could additionally include RooBinSamplingPdf and share implementation with RooBinningCategory.
This larger version would also make it possible for HistFactory models to explicitly share one bin-index node between several bin-wise functions.
I don't think this necessarily needs to block #23298.
The RooBinWidthFunction changes in that PR make the duplicated bin-index computation particularly visible, but a genuinely reusable RooBinIndex requires decisions about binning ownership, index ordering, categories, dynamic binnings and evaluation backends that go beyond the scope of the original PR.
One reasonable path would therefore be to merge #23298 as-is and use this issue as the basis for a later refactor.
Alternatively, if there is agreement on the desired RooBinIndex abstraction, #23298 could serve as the first consumer of it.
ROOT version
latest master
Installation method
built from source
Operating system
linux
Additional context
One of the core motivations for this is to make the computational graph more aligned with the client/server proxy links which form the basis of HS3 export.
Explain what you would like to see improved and how.
Related PRs: #23298, #15543, #11171, #12955
Motivation
While working on #23298, the question came up whether the computation of a histogram bin index should be represented by a standalone RooFit object, tentatively called
RooBinIndex.The immediate motivation is
RooBinWidthFunction: after #23298 it depends directly on its observables, but consequently needs to determine the current multi-dimensional bin itself in both scalar and batch evaluation. This logic is not specific to bin widths, however. Similar coordinate-to-bin-index computations already exist in several RooFit and HistFactory classes.A first-class
RooBinIndexcould provide a single graph node representingand allow multiple histogram-like objects with the same binning to share that result.
This issue is intended to collect the relevant use cases and design questions. It could either serve as the starting point for a refactor now, or remain as follow-up work after merging #23298 independently.
Existing implementations / potential consumers
The following classes currently contain or rely on closely related bin-index logic:
RooBinWidthFunctionThe implementation in #23298 computes the current bin directly from its observable list, including multi-dimensional index flattening and separate scalar/batch implementations.
This is the immediate use case that prompted the discussion.
RooHistFuncRooHistFunccurrently haswhich effectively implement a private version of the proposed abstraction.
Its evaluation code additionally has separate scalar, 1D batch, and ND batch paths for mapping function observables onto the underlying histogram bins.
RooHistPdfRooHistPdfperforms essentially the same mapping from PDF observables to the bins of itsRooDataHist.For zero-order histogram evaluation this could naturally become
Interpolation would still require the actual coordinates and neighbouring bin geometry, so
RooBinIndexwould only represent the central-bin lookup.RooParamHistFuncThis is perhaps the simplest potential consumer. Its scalar evaluation currently amounts to
idx = dataHist.getIndex(observables); return parameters[idx];while code generation independently asks
RooDataHistto generate the corresponding tree-index expression.This could naturally become a parameter lookup driven by a
RooBinIndex.HistFactory
ParamHistFuncThe HistFactory implementation contains several versions of the same operation:
RooDataHist::getIndex();RooAbsBinning::binNumbers()and explicit index multipliers;RooDataHist::calculateTreeIndexForCodeSquash().This is a strong candidate for consolidation.
PiecewiseInterpolation#15543 already introduced an optimization based on the fact that the nominal and variation
RooHistFuncs in HistFactory use the same bin structure.The generated code computes the bin index only once and then uses it to address the nominal, low, and high arrays.
A first-class
RooBinIndexcould make this sharing explicit in the RooFit computation graph instead of rediscovering it in code generation.Conceptually, a HistFactory channel could then look like
RooDataHistRooDataHist::calcTreeIndex()is currently the main general implementation of multi-dimensional bin-index calculation, andcalculateTreeIndexForCodeSquash()implements the corresponding generated expression.It probably should share an implementation with
RooBinIndex, butRooDataHistitself should not necessarily depend on a graph node: it also needs to calculate indices for arbitrary coordinate collections outside a computation graph.A possible layering would therefore be
Secondary candidates
RooBinSamplingPdfalso has duplicated scalar/batch bin lookup: scalar evaluation usesgetBin(), while batch evaluation independently determines the bin from the boundaries.RooBinningCategoryis conceptually very close to a one-dimensionalRooBinIndex: it maps a real l-value through a named binning onto a category whose index corresponds to the bin number. It probably should not disappear, since the category labels are useful, but the two implementations might eventually share their underlying bin-lookup machinery.Other uses of
binNumber()such as inRooFFTConvPdf,RooPlot, orRooMomentMorphFuncNDappear to be local geometry/cache algorithms rather than reusable graph-level bin-index computations, so they probably do not need to participate in this refactor.Design questions
A reusable
RooBinIndexturns out to require a few architectural decisions.1. Which binning defines the index?
For
RooBinWidthFunctionafter #23298, the live observables and their current default binnings are authoritative.For
RooHistFuncandRooHistPdf, however, the values can come from one set of observables while the relevant bin geometry comes from the corresponding histogram observables /RooDataHist.Therefore an API that only stores
RooBinIndex(name, observables);and always uses
observables[i].getBinning()may be too restrictive.The value source and the binning source may need to be representable separately.
2. What is the multi-dimensional flattening convention?
RooDataHistand HistFactory do not universally use the same ordering.RooDataHistindexing makes the last listed dimension the fastest-running one.The legacy HistFactory
ParamHistFunc, on the other hand, stores parameters in TH1 ordering, where the first/x dimension is fastest, and contains explicit conversion logic between the two conventions.A standalone
RooBinIndexshould have a clearly defined canonical ordering. Consumers with legacy storage conventions could then transform their arrays explicitly if necessary.3. How are categories handled?
For categories, the histogram-bin number is the ordinal state number, which is not necessarily identical to the category's numeric state index.
Scalar evaluation can use
RooAbsCategoryLValue::getBin(), but batch evaluation sees the numeric category values and therefore needs a proper value-to-ordinal mapping.This needs to be handled centrally if
RooBinIndexis intended to support mixed real/category histograms.4. Dynamic and named binnings
The implementation should define how it behaves when
This is already relevant in #23298 because
RooBinWidthFunctionneeds to detect binning changes that do not necessarily propagate through the usual dirty-state mechanism.5. Evaluation backends
If introduced as a RooFit graph node,
RooBinIndexshould ideally have consistent support forEvalContextbatch evaluation;Otherwise it risks simply moving the current duplication to a different layer.
Possible scope
There seem to be two reasonable levels of implementation.
Minimal extraction
Introduce
RooBinIndexprimarily for #23298 and the immediately duplicated helpers:RooBinIndex;RooBinWidthFunction;RooHistFunc::getBin()/getBins()where appropriate.This would be a relatively small follow-up and would not require restructuring histogram evaluation more generally.
Canonical bin-index node
Make
RooBinIndexthe common graph-level representation of coordinate-to-bin mapping and adapt at least:RooBinWidthFunction;RooHistFunc;RooHistPdf;RooParamHistFunc;ParamHistFunc;PiecewiseInterpolation;RooDataHisthelper implementation.A more exhaustive cleanup could additionally include
RooBinSamplingPdfand share implementation withRooBinningCategory.This larger version would also make it possible for HistFactory models to explicitly share one bin-index node between several bin-wise functions.
Relation to #23298
I don't think this necessarily needs to block #23298.
The
RooBinWidthFunctionchanges in that PR make the duplicated bin-index computation particularly visible, but a genuinely reusableRooBinIndexrequires decisions about binning ownership, index ordering, categories, dynamic binnings and evaluation backends that go beyond the scope of the original PR.One reasonable path would therefore be to merge #23298 as-is and use this issue as the basis for a later refactor.
Alternatively, if there is agreement on the desired
RooBinIndexabstraction, #23298 could serve as the first consumer of it.ROOT version
latest master
Installation method
built from source
Operating system
linux
Additional context
One of the core motivations for this is to make the computational graph more aligned with the client/server proxy links which form the basis of HS3 export.