[RF] Make RooBinWidthFunction depend directly on observables - #23298
Open
cburgard wants to merge 3 commits into
Open
[RF] Make RooBinWidthFunction depend directly on observables#23298cburgard wants to merge 3 commits into
cburgard wants to merge 3 commits into
Conversation
Cover multidimensional volumes and inverse volumes, evaluation and bin-volume caching, plotting hints, disable/compile behavior, and workspace persistence through the existing RooHistFunc constructor. Include a version-1 ROOT fixture and its regeneration recipe for the schema migration tests. Register the tests with the existing RooFitCore test target. Validation: all six tests pass against the available version-1 ROOT build.
Replace the histogram proxy with a RooListProxy and retain the RooHistFunc constructor as a forwarding compatibility overload. Use the live observables' default binnings, including when they differ from the data histogram's bins. Keep cached bin volumes and RooFit value caching, and implement scalar/batch bin lookup and plotting hints without a histogram dependency. Add version-1 schema evolution with deferred proxy repair for shared histogram references. Extend regression coverage for the new API, cache invalidation, observable redirection, histogram lifetime independence, and migrated workspaces. Update the HS3 importer/exporter to use variables instead of the removed histFunc() accessor so the intermediate commit remains buildable. Continue accepting legacy histogram-reference JSON. Validation: 12 RooBinWidthFunction tests, source compilation, and the full ROOT build and HS3/HistFactory integration suites for this implementation.
Check numerical round trips with live-variable binning that differs from the histogram binning. Verify the variables representation and keep binning metadata in domains rather than in the bin-volume function. Add an inline multidimensional domains-only fixture and regression coverage for importing legacy histogram-reference JSON. Validation: all 73 HS3 tests pass, including the three bin-volume tests.
cburgard
requested review from
bellenot,
guitargeek and
hageboeck
as code owners
September 8, 2026 07:51
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
2 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.
This Pull request:
Changes or fixes:
At the DEMOS meeting on September 4, 2026, the current implementation of
RooBinWidthFunctionwas identified as an obstacle to clean HS3 export: the object references a histogram function rather than directly storing its actual inputs—the observables.This PR removes that indirection in both RooFit and HS3.
RooBinWidthFunctionnow stores an observable list and computes bin volumes from the observables’ default binnings. HS3 represents those inputs through a"variables"list, with binning restored from"domains".The changes:
RooArgListconstructor while retaining the legacyRooHistFuncconstructor.Validation includes a successful full ROOT build and 131 passing tests covering
RooBinWidthFunction, HS3, HistFactory, and component plotting. The initial regression-test commit also passes against the version-1 implementation.The meeting also raised the possibility of extracting bin-index computation into a standalone RooFit class. This PR does not preclude that work: depending on the developers’ preference, it can be merged independently or serve as the baseline for that further refactoring.
Checklist:
This PR fixes #