[PWGHF] HFInvMassFitter: stabilize fit quality, add fit-quality status to the output - #17348
Open
lubynets wants to merge 34 commits into
Open
[PWGHF] HFInvMassFitter: stabilize fit quality, add fit-quality status to the output#17348lubynets wants to merge 34 commits into
lubynets wants to merge 34 commits into
Conversation
lubynets
requested review from
NicoleBastid,
alibuild,
apalasciano,
deepathoms,
fcatalan92,
fcolamar,
fgrosa,
gluparel,
hahassan7,
jpxrk,
mfaggin,
mmazzilli,
singhra1994,
stefanopolitano,
vkucera,
xinyepeng and
zhangbiao-phy
as code owners
August 5, 2026 15:43
|
O2 linter results: ❌ 0 errors, |
github-actions
Bot
changed the title
vkucera
reviewed
Aug 5, 2026
Comment on lines
-498
to
+526
| auto setFixedValue = [&iSliceVar](bool const& isFix, std::vector<double> const& fixManual, const TH1* histToFix, std::function<void(double)> setFunc, std::string const& var) -> void { | ||
| auto setFixedValue = [&iSliceVar, massFitter](bool const& isFix, std::vector<double> const& fixManual, const TH1* histToFix, void (HFInvMassFitter::*setter)(double), std::string const& var) -> void { |
Collaborator
There was a problem hiding this comment.
What is the motivation for removing std::function here?
Contributor
Author
There was a problem hiding this comment.
I wanted to make setFixedValue() uniform with setDscbParameter()
Collaborator
There was a problem hiding this comment.
OK, thanks. What speaks against using std::function everywhere?
Contributor
Author
There was a problem hiding this comment.
No special reasoning against std::function, it was a random choice. Are there reasons not to use void()()?
Collaborator
There was a problem hiding this comment.
No special reasoning against
std::function, it was a random choice. Are there reasons not to usevoid()()?
Isn't it some kind of old unsafe C-style syntax that has been superseded by stuff in the standard library?
Collaborator
|
Error while checking build/O2Physics/code-check for 0336f86 at 2026-08-05 20:47: Full log here. |
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
3 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.
1. Fit quality stabilization
With the existing version of the fitter some fits are exceptionally poor, and the shape of background and / or peak is completely wrong. It happens due to wrong estimation of initial values of background's and signal's integral. In this PR:
-- Estimate of background and signal integral was fixed;
-- Pre-fit of background sidebands is now done with
chi2FitTo()always, sincefitTo()function (likelihood) turned out to be agnostic to theRanges()parameter, i.e. it also fitted the peak when sidebands were requested (that's why at some fit plots the gray line representing sidebands pre-fit was sometimes completely misfittig, while the total fit looked healthy).2. Add fit-quality status to the output
--
hFitResulthistogram is added to the output, which contains RooFit'sstatus, covariance matrix quality (cov qual), estimated distance to minimum (edm), minimum value of the negative log-likelihood (minNLL) and global correlation coefficient of the signal's integral with other fit parameters (N Sig GCC);--
CovCorrMatrices/hCovCorrMatrixhistograms are added, which contain covariance and correlation across all fit parameters.3. Code cleaning
-- Avoided crash if a single histogram fitting is failed. Instead the exception is handled and fitting of remaining histograms continues;
-- Fixed (most of) clang-tidy and cpp check errors.