Skip to content

Conversation

@lubynets
Copy link
Contributor

@lubynets lubynets commented Dec 23, 2025

Bugfix:

  • Before the json configurable UseLikelihood did not have any effect: independently on its value true or false, always likelihood fit was performed. This bug is fixed now. Note: it seems that in Chi2 mode (when RooAbsPdf::chi2FitTo() function is called) setting the fitting range has no effect. This problem is to be investigated, any help will be highly appreciated;

Code cleaning and refactoring:

  • Reduced code repetition in several places;
  • Used enums instead of magic values for fit functions and config parameters;
  • Used std::array instead of std::vectors when the size is known at the compilation time;
  • ROOT types replaced with standard c++ types (with an exception for TString in several places);
  • Got rid of compilation warnings;
  • Deleted the default constructor UPD: since it was not used and served exclusively as an entry point for running the fitter as a Root macro. Now the instructions how to run the fitter as a Root macro are fixed and do not require default constructor as an entry point any more;
  • Factored out into a function:
    • open file and check for nullptr;
    • get object from file and check for nullptr;
    • read variables and vectors from json configurables; avoid crash when the field is optional and crash with explicit error message when the field is mandatory;
    • check whether the vector of configurables has the same size as number of fitting ranges (or zero if the vector is optional);
  • Got rid of unnecessary histograms cloning when passing them from file to the fitter and when rebinning them;
  • Function implementations, which occupy more than 1 line, moved from header to the source file.

@github-actions github-actions bot added the pwghf PWG-HF label Dec 23, 2025
@github-actions github-actions bot changed the title HFInvMassFitter: code cleaning and refactoring [PWGHF] HFInvMassFitter: code cleaning and refactoring Dec 23, 2025
@github-actions
Copy link

github-actions bot commented Dec 23, 2025

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

@vkucera
Copy link
Collaborator

vkucera commented Dec 24, 2025

Hi @lubynets , thanks for the extensive improvement. Can you please also fix the issues reported in the bug tracker?

@lubynets
Copy link
Contributor Author

Hi @lubynets , thanks for the extensive improvement. Can you please also fix the issues reported in the bug tracker?

Done.

@vkucera
Copy link
Collaborator

vkucera commented Dec 27, 2025

Hi @lubynets , thanks for the extensive improvement. Can you please also fix the issues reported in the bug tracker?

Done.

Thanks but it doesn't seem you fixed the issues found by Clang-Tidy, e.g. setNumberOfSigmaForSidebands still assigns double to int.

@lubynets
Copy link
Contributor Author

lubynets commented Dec 27, 2025

Hi @lubynets , thanks for the extensive improvement. Can you please also fix the issues reported in the bug tracker?

Done.

Thanks but it doesn't seem you fixed the issues found by Clang-Tidy, e.g. setNumberOfSigmaForSidebands still assigns double to int.

Indeed, there is such an issue. However I cannot read it at this page. For me the list of issues ends up at DPG/Tasks/AOTTrack/PID/HMPID/hmpidQa.cxx. And when I click on any link in the table of content below the DPG, I cannot access it (the same applies to PWGHF). Is it a limitation of how github page is shown to user? (sorry for probably naive question, but it persists e.g. with another browser).
UPD: found a tip with .md in the end of url.

@lubynets
Copy link
Contributor Author

Hi @lubynets , thanks for the extensive improvement. Can you please also fix the issues reported in the bug tracker?

Done.

Thanks but it doesn't seem you fixed the issues found by Clang-Tidy, e.g. setNumberOfSigmaForSidebands still assigns double to int.

Now it should be fixed (sorry in advance if I overlooked something).

@vkucera
Copy link
Collaborator

vkucera commented Dec 28, 2025

Hi @lubynets , thanks for the extensive improvement. Can you please also fix the issues reported in the bug tracker?

Done.

Thanks but it doesn't seem you fixed the issues found by Clang-Tidy, e.g. setNumberOfSigmaForSidebands still assigns double to int.

Indeed, there is such an issue. However I cannot read it at this page. For me the list of issues ends up at DPG/Tasks/AOTTrack/PID/HMPID/hmpidQa.cxx. And when I click on any link in the table of content below the DPG, I cannot access it (the same applies to PWGHF). Is it a limitation of how github page is shown to user? (sorry for probably naive question, but it persists e.g. with another browser). UPD: found a tip with .md in the end of url.

Yes, that's why there is this below the title:

To access the full raw file, add .md in the URL.

@vkucera
Copy link
Collaborator

vkucera commented Dec 28, 2025

Hi @lubynets , thanks for the extensive improvement. Can you please also fix the issues reported in the bug tracker?

Done.

Thanks but it doesn't seem you fixed the issues found by Clang-Tidy, e.g. setNumberOfSigmaForSidebands still assigns double to int.

Now it should be fixed (sorry in advance if I overlooked something).

Thanks!

@vkucera
Copy link
Collaborator

vkucera commented Jan 8, 2026

@pstahlhu Since you also contributed to this code recently, can you please have a look?

@alibuild
Copy link
Collaborator

Error while checking build/O2Physics/o2 for b89f95f at 2026-01-10 04:09:

[1545/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-bplus-mcp-charged.dir/jetFinderBplusMCPCharged.cxx.o
[1546/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-bplus-mcd-charged
[1547/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-bplus-mcp-charged
[1548/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-xictoxipipi-data-charged.dir/jetFinderXicToXiPiPiDataCharged.cxx.o
[1549/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-v0-data-charged.dir/jetFinderV0DataCharged.cxx.o
[1550/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-xictoxipipi-data-charged
[1551/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-xictoxipipi-mcd-charged.dir/jetFinderXicToXiPiPiMCDCharged.cxx.o
[1552/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-v0-data-charged
[1553/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-xictoxipipi-mcd-charged
[1554/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-mc-signal-loss.dir/mcSignalLoss.cxx.o
[1555/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-xictoxipipi-mcp-charged.dir/jetFinderXicToXiPiPiMCPCharged.cxx.o
[1556/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-v0-mcp-charged.dir/jetFinderV0MCPCharged.cxx.o
[1557/2849] Linking CXX executable stage/bin/o2-analysis-lf-mc-signal-loss
[1558/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-its-tpc-matching-qa.dir/lfITSTPCMatchingQA.cxx.o
[1559/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-xictoxipipi-mcp-charged
[1560/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-v0-mcp-charged
[1561/2849] Linking CXX executable stage/bin/o2-analysis-lf-its-tpc-matching-qa
[1562/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-kfperformancestudy.dir/kfPerformanceStudy.cxx.o
[1563/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-v0-mcd-charged.dir/jetFinderV0MCDCharged.cxx.o
[1564/2849] Linking CXX executable stage/bin/o2-analysis-lf-kfperformancestudy
[1565/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-v0-mcd-charged
[1566/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-track-checks.dir/trackchecks.cxx.o
[1567/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-dielectron-mcd-charged.dir/jetFinderDielectronMCDCharged.cxx.o
[1568/2849] Linking CXX executable stage/bin/o2-analysis-lf-track-checks
[1569/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-dielectron-mcd-charged
[1570/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-lfpropstudy.dir/lfpropStudy.cxx.o
[1571/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-vertexqa.dir/vertexQA.cxx.o
[1572/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-d0d0bar-data-charged.dir/jetFinderD0D0BarDataCharged.cxx.o
[1573/2849] Linking CXX executable stage/bin/o2-analysis-lf-lfpropstudy
[1574/2849] Linking CXX executable stage/bin/o2-analysis-lf-vertexqa
[1575/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-d0d0bar-data-charged
[1576/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-pid-qa.dir/lfpidqa.cxx.o
[1577/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-dielectron-data-charged.dir/jetFinderDielectronDataCharged.cxx.o
[1578/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-strangeness-tracking-qc.dir/strangenessTrackingQC.cxx.o
[1579/2849] Linking CXX executable stage/bin/o2-analysis-lf-pid-qa
[1580/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-dielectron-data-charged
[1581/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-dielectron-mcp-charged.dir/jetFinderDielectronMCPCharged.cxx.o
[1582/2849] Linking CXX executable stage/bin/o2-analysis-lf-strangeness-tracking-qc
[1583/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-strangenessqcpp.dir/strangenessQCPP.cxx.o
[1584/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-dielectron-mcp-charged
[1585/2849] Linking CXX executable stage/bin/o2-analysis-lf-strangenessqcpp
[1586/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-d0d0bar-mcd-charged.dir/jetFinderD0D0BarMCDCharged.cxx.o
[1587/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-d0d0bar-mcp-charged.dir/jetFinderD0D0BarMCPCharged.cxx.o
[1588/2849] Building CXX object PWGJE/JetFinders/CMakeFiles/O2Physicsexe-analysis-je-jet-finder-dplusdminus-mcd-charged.dir/jetFinderDplusDminusMCDCharged.cxx.o
[1589/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-tpc-dedx-postcalibration.dir/tpc_dEdx_postcalibration.cxx.o
[1590/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-d0d0bar-mcd-charged
[1591/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-d0d0bar-mcp-charged
[1592/2849] Building CXX object PWGLF/Tasks/QC/CMakeFiles/O2Physicsexe-analysis-lf-mcinelgt0.dir/mcinelgt0.cxx.o
[1593/2849] Linking CXX executable stage/bin/o2-analysis-je-jet-finder-dplusdminus-mcd-charged
[1594/2849] Linking CXX executable stage/bin/o2-analysis-lf-tpc-dedx-postcalibration

Full log here.

@lubynets
Copy link
Contributor Author

@pstahlhu Since you also contributed to this code recently, can you please have a look?

Hi @vkucera, Phil had left the collaboration and probably will not look at the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pwghf PWG-HF

Development

Successfully merging this pull request may close these issues.

3 participants