[PWGHF] Improvements to HFInvMassFitter #13002

pstahlhu · 2025-09-16T16:22:35Z

@lubynets

Fix canvas division for the plotting of mass fits and the corresponding residuals
Add fit/data histograms to estimate fit quality
Add possibility to constrain the relative fraction of the two Gaussians in a Double Gaussian signal function
Store second sigma and the relative fraction of double Gaussians to output file, so that they can be analyzed and possibly constrained in a following iteration

github-actions · 2025-09-16T16:22:49Z

O2 linter results: ❌ 1 errors, ⚠️ 0 warnings, 🔕 0 disabled

PWGHF/D2H/Macros/HFInvMassFitter.cxx

PWGHF/D2H/Macros/HFInvMassFitter.h

PWGHF/D2H/Macros/HFInvMassFitter.cxx

PWGHF/D2H/Macros/runMassFitter.C

lubynets · 2025-09-18T22:23:35Z

Hi @pstahlhu,
thanks for the development! The PR generally looks fine to me with some comments and a PR which I sent to your branch.

lubynets

Hi @pstahlhu,
Thanks for the development and addressing my comments! The PR looks fine for me with two comments:

can you recover the $S_{\text{count}}$ in the statistics table?
if you manage generally to improve the quality of printing of the statistics table on the canvas (both interactive and pdf), such as alignment, relative size of font to sub-canvas size, etc. , that's will be nice. But I understand that it's tough to foreseen different canvas size divisions, so maybe there is no universal solution...

lubynets · 2025-10-21T17:19:45Z

Hi @pstahlhu, could you maybe resolve conflicts with target branch?
BTW, concerning the $S_{\mathrm{count}}$ value - in case of double Gaus does the algorithm ensure that the greater sigma will be used for its computation?

lubynets · 2025-11-03T23:19:43Z

Hi @pstahlhu, could you maybe resolve conflicts with target branch? BTW, concerning the S count value - in case of double Gaus does the algorithm ensure that the greater sigma will be used for its computation?

Hi @pstahlhu, let me gently remind to resolve the conflict. It is basically resolved by me, I hope this PR (or if you prefer an explicit commit) you'll find useful. If you have any questions about it, feel free to ask.

vkucera · 2025-11-12T17:07:58Z

@pstahlhu @lubynets What is the status?

lubynets · 2025-11-12T17:20:34Z

@pstahlhu @lubynets What is the status?

From my side the PR is fine, although the last force push discards some clang-tidy fixes done by you, @vkucera. I propose one of the three solutions:

@pstahlhu to fix the mentioned discarding;
I have a branch with the same content as Phil's PR but conflict is resolved via merge, not rebase - I can submit a new PR;
leave as is - I have plans to refactor the fitter and enhance its functionality, so I'll be able to revert the discarded clang-tidy fixes later.

pstahlhu · 2025-11-13T20:04:39Z

Hi @vkucera @lubynets, thanks for following up on this.
I made sure that I did not change Vit's clang-tidy fixes from the previous PRs and also comply with them in my new implementation, and I also tried to make some code look better visually.
From my side, the PR is ready to be merged now. Could you please have a look?
Thanks a lot!

lubynets · 2025-11-14T14:26:15Z

Hi @vkucera @lubynets, thanks for following up on this. I made sure that I did not change Vit's clang-tidy fixes from the previous PRs and also comply with them in my new implementation, and I also tried to make some code look better visually. From my side, the PR is ready to be merged now. Could you please have a look? Thanks a lot!

Hi @pstahlhu, thanks for updating the PR! ~~For me it is fine.~~ There is one comment, could you please look at it?

lubynets · 2025-11-14T14:47:46Z

PWGHF/D2H/Macros/runMassFitter.C

      setFixedValue(fixMean, fixMeanManual, hMeanToFix, std::bind(&HFInvMassFitter::setFixGaussianMean, massFitter, std::placeholders::_1), "MEAN");
      setFixedValue(fixSigma, fixSigmaManual, hSigmaToFix, std::bind(&HFInvMassFitter::setFixGaussianSigma, massFitter, std::placeholders::_1), "SIGMA");
      setFixedValue(fixSecondSigma, fixSecondSigmaManual, hSecondSigmaToFix, std::bind(&HFInvMassFitter::setFixSecondGaussianSigma, massFitter, std::placeholders::_1), "SECOND SIGMA");
+      setFixedValue(fixFracDoubleGaus, fixFracDoubleGausManual, nullptr, std::bind(&HFInvMassFitter::setFixFrac2Gaus, massFitter, std::placeholders::_1), "FRAC DOUBLE GAUS");


Doesn't it crash here due to nullptr as the third argument? The lambda-function does not foresee this case explicitly.

Yep, definitely a null pointer dereference there.

In general, I don't understand why there are raw pointers all over the place. They should be replaced with smart pointers or just with normal stack allocations.

Hi @lubynets, thanks for spotting this error! I think that it should be fixed now. Do you agree?

Thanks for fix! Yes, now it should be fine.

pstahlhu · 2025-11-17T16:06:39Z

@vkucera Could you please merge this PR if you think that it's ready to merge? Thanks!

vkucera · 2025-11-17T17:45:41Z

@vkucera Could you please merge this PR if you think that it's ready to merge? Thanks!

@zhangbiao-phy @stefanopolitano

zhangbiao-phy

Thanks @pstahlhu @lubynets and @vkucera!

Co-authored-by: Oleksii Lubynets <O.Lubynets@gsi.de>

pstahlhu requested review from NicoleBastid, alibuild, apalasciano, deepathoms, fcatalan92, fcolamar, fgrosa, gluparel, hahassan7, jpxrk, mfaggin, mmazzilli, stefanopolitano, vkucera and zhangbiao-phy as code owners September 16, 2025 16:22

github-actions bot added the pwghf PWG-HF label Sep 16, 2025

github-actions bot changed the title ~~[PWGHF]: Improvements to HFInvMassFitter~~ [PWGHF] Improvements to HFInvMassFitter Sep 16, 2025

lubynets reviewed Sep 18, 2025

View reviewed changes

PWGHF/D2H/Macros/HFInvMassFitter.cxx Show resolved Hide resolved

lubynets reviewed Sep 18, 2025

View reviewed changes

PWGHF/D2H/Macros/HFInvMassFitter.h Outdated Show resolved Hide resolved

lubynets reviewed Sep 18, 2025

View reviewed changes

PWGHF/D2H/Macros/HFInvMassFitter.cxx Outdated Show resolved Hide resolved

lubynets reviewed Sep 18, 2025

View reviewed changes

PWGHF/D2H/Macros/runMassFitter.C Show resolved Hide resolved

pstahlhu marked this pull request as draft September 19, 2025 12:59

pstahlhu requested a review from lubynets September 19, 2025 15:39

pstahlhu marked this pull request as ready for review September 19, 2025 15:40

lubynets reviewed Sep 22, 2025

View reviewed changes

vkucera marked this pull request as draft October 24, 2025 13:36

pstahlhu and others added 9 commits November 7, 2025 13:37

HFInvMassFitter: Fix canvas division, add ratio histograms

19475fa

Improve handling of second sigma for double Gaussian fits

5488306

Store fraction for double Gaussian and add possibility to fix it

b464a70

Store signal parameters to hist only when present

92823fc

Improve plotting style of the mass canvases

56db18f

Implement Oleksii's comments

0db0b7f

adapt config_massfitter for FixFracDoubleGaus

4a19c46

Fix includes in runMassFitter.C

be724be

Restore S_count metric to mass fit histograms

02f1bba

pstahlhu force-pushed the master branch from cb32b57 to 02f1bba Compare November 7, 2025 12:44

pstahlhu marked this pull request as ready for review November 7, 2025 12:48

Comply with Vit's clang-tidy fixes

e0cd945

Unify histogram order and fix if condition for plot labels

f61f7be

lubynets reviewed Nov 14, 2025

View reviewed changes

Fix lambda to fix fraction of double Gaussian

6f24caa

zhangbiao-phy approved these changes Nov 17, 2025

View reviewed changes

zhangbiao-phy merged commit 68d5234 into AliceO2Group:master Nov 17, 2025
14 of 15 checks passed

lmattei01 pushed a commit to lmattei01/O2Physics that referenced this pull request Dec 5, 2025

[PWGHF] Improvements to HFInvMassFitter (AliceO2Group#13002)

4af760b

Co-authored-by: Oleksii Lubynets <O.Lubynets@gsi.de>

[PWGHF] Improvements to HFInvMassFitter #13002

[PWGHF] Improvements to HFInvMassFitter #13002

Uh oh!

Conversation

pstahlhu commented Sep 16, 2025

Uh oh!

github-actions bot commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lubynets commented Sep 18, 2025

Uh oh!

lubynets left a comment

Choose a reason for hiding this comment

Uh oh!

lubynets commented Oct 21, 2025

Uh oh!

lubynets commented Nov 3, 2025

Uh oh!

vkucera commented Nov 12, 2025

Uh oh!

lubynets commented Nov 12, 2025

Uh oh!

pstahlhu commented Nov 13, 2025

Uh oh!

lubynets commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lubynets Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

vkucera Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

vkucera Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

pstahlhu Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

lubynets Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

pstahlhu commented Nov 17, 2025

Uh oh!

vkucera commented Nov 17, 2025

Uh oh!

zhangbiao-phy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Sep 16, 2025 •

edited

Loading

lubynets commented Nov 14, 2025 •

edited

Loading