fix: Gaussian histogram fit implementation in Core - #5831
Draft
benjaminhuth wants to merge 5 commits into
Draft
Conversation
github-actions
Bot
added
Component - Core
Contributor
Public API surface diff+8 added, 5 breaking.
|
Contributor
benjaminhuth
changed the title
benjaminhuth
force-pushed
the
feature/resolution-fit-without-root
branch
from
August 6, 2026 08:00
52610d3 to
898696d
Compare
Member
|
Honestly I'm not sure this should be in core. I'm assuming this is for the python use case? Can't we fit in python directly with scikit? |
Member
Author
|
Hmm python based fit would be an option, but then we would have divergence between the ROOT and Python based metrics. The nice thing about having it in Core would be, we can unit-test equivalence against ROOT, which is our correctness reference. |
Member
Author
|
(okay, but we could in principle also check scikit fit against root in our CI...) |
Member
|
Right but it expands our footprint into functionality that's not really meant to be covered by the library. The histogram library is already pushing this line a bit, but mostly wraps boost-histogram |
…ance Introduces HistogramFitFunction, a std::function-based abstraction over "fit a Gaussian to a histogram, optionally in a range", so the resolution fit backend used by TrackFitterPerformanceCollector is pluggable rather than hard-coded. ActsPlugins::RootHistogramFit (ROOT's TH1::Fit) is the only backend provided here; a ROOT-free C++ implementation follows in a later change, and a Python callable (e.g. scipy.optimize.curve_fit) already works today via pybind11's functional.h conversion -- no C++ changes needed on that side. TrackFitterPerformanceCollector now owns fitProfiles() (mean/width profile extraction via the pluggable fit backend), absorbing logic that both RootTrackFitterPerformanceWriter and PythonTrackFitterPerformanceWriter used to duplicate. Config::fitFunction defaults to an empty std::function; if left unset, fitProfiles() logs a warning and returns no profiles instead of crashing. RootTrackFitterPerformanceWriter hard-wires RootHistogramFit as its own backend, since it already depends on ROOT. Python's PythonTrackFitterPerformanceWriter.Config.fitFunction has no default -- callers must pick a backend explicitly. Also adds ValueHistogram (a Histogram-like type for derived per-bin values/errors, e.g. a fitted mean or width) and generic histogram infrastructure (setBinContent/binContent, sliceLastAxis, totalContent) needed by the fit interface, plus their Python bindings and toRoot() converters. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013s1fSXsr4v6mPs8DcmezD4
…on-fit-without-root
benjaminhuth
force-pushed
the
feature/resolution-fit-without-root
branch
from
August 10, 2026 17:11
898696d to
446fce2
Compare
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.
No description provided.