Skip to content

[PWGLF] Fix Omega2012 and K1 MC efficiency denominators and truth matching - #18431

Open
BongHwi wants to merge 3 commits into
AliceO2Group:masterfrom
BongHwi:omega2012-mc-selected-denominator
Open

BongHwi wants to merge 3 commits into
AliceO2Group:masterfrom
BongHwi:omega2012-mc-selected-denominator

Conversation

@BongHwi

@BongHwi BongHwi commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

This updates the Omega2012 and K1 MC analysis and export paths so efficiency numerators can be compared with generated mothers from the same selected events and rapidity range.

For Omega2012:

  • Add selected-event generated denominators and reconstructed response histograms for the XiK0s and Xi1530K channels.
  • Match reconstructed candidates to the parent table and apply consistent truth-rapidity, kinematic, and optional injected-generator selections.
  • Share event-selection defaults between the analysis and ML exporter.
  • Keep generated mothers from selected events even when no candidate is reconstructed. The generated rapidity cut is independent of the loose candidate cut.

For K1:

  • Use collision-local parent matching in both full-track and micro-track MC processing.
  • Fill generated pT and rapidity in the ML truth table.
  • Apply the parent rapidity cut to MC efficiency numerators by default, and add reconstructed-versus-generated pT response histograms with matching generated distributions.
  • Preserve event-selection defaults, candidate features, and pass bits. Valid parent matches use the true mother PDG; unavailable parent kinematics remain NaN.

K1 reconstructed MC processing now requires ResoMCParents_001, including when generated processing is disabled. The existing MCReco/pt histogram remains a QA distribution; efficiency calculations should use the new response histograms.

Bong-Hwi Lim added 2 commits October 23, 2026 18:26
Add selected-event generated denominators and reconstructed response
histograms for the XiK0s and Xi1530K channels. Match reconstructed mothers
to the parent table and apply consistent truth-rapidity, kinematic and
optional injected-generator selections.

Share Omega2012 event-selection defaults between analysis and ML export.
Use an independent generated-mother rapidity limit so the audit includes
selected events without candidates while preserving the loose candidate
rapidity acceptance.
@github-actions

Copy link
Copy Markdown

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

@alibuild

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/code-check for 132b8b8 at 2026-10-23 14:41:

## sw/BUILD/O2Physics-code-check-latest/log
--
========== List of issues found ==========
++ echo 'To reproduce the results below locally, run: '\''aliBuild build O2Physics-code-check -e ALIBUILD_BASE_HASH=upstream/master'\''.'
To reproduce the results below locally, run: 'aliBuild build O2Physics-code-check -e ALIBUILD_BASE_HASH=upstream/master'.
++ echo 'To apply all automatic fixes, add the '\''-e O2PHYSICS_CHECKER_FIX=1'\'' option.'
To apply all automatic fixes, add the '-e O2PHYSICS_CHECKER_FIX=1' option.
++ echo 'See the documentation for more details at: <https://aliceo2group.github.io/analysis-framework/docs/tools/#running-with-o2physics-code-check>.'
See the documentation for more details at: <https://aliceo2group.github.io/analysis-framework/docs/tools/#running-with-o2physics-code-check>.
++ echo 'Found 3 errors and 7 warnings.'
Found 3 errors and 7 warnings.
++ [[ 3 -gt 0 ]]
++ cat /sw/BUILD/b595739ac09ae99f08973fb66308604e1da917fc/O2Physics-code-check/errors.txt
PWGLF/Tasks/Resonances/omega2012Analysis.cxx:701:40: error: parameter 'xi' is unused [misc-unused-parameters,-warnings-as-errors]
PWGLF/Tasks/Resonances/omega2012Analysis.cxx:701:56: error: parameter 'pion' is unused [misc-unused-parameters,-warnings-as-errors]
PWGLF/Tasks/Resonances/omega2012Analysis.cxx:701:74: error: parameter 'kaon' is unused [misc-unused-parameters,-warnings-as-errors]
++ [[ 7 -gt 0 ]]
++ cat /sw/BUILD/b595739ac09ae99f08973fb66308604e1da917fc/O2Physics-code-check/warnings.txt
PWGLF/DataModel/LFK1MlTables.h:66:56: warning: do not declare C-style arrays, use 'std::array' instead [modernize-avoid-c-arrays]
PWGLF/Tasks/Resonances/omega2012TrainingTable.cxx:277:27: warning: redundant explicit casting to the same type 'int64_t' (aka 'long') as the sub-expression, remove this casting [readability-redundant-casting]
PWGLF/Tasks/Resonances/omega2012TrainingTable.cxx:292:44: warning: redundant explicit casting to the same type 'int64_t' (aka 'long') as the sub-expression, remove this casting [readability-redundant-casting]
PWGLF/Tasks/Resonances/omega2012TrainingTable.cxx:314:26: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
PWGLF/Tasks/Resonances/omega2012TrainingTable.cxx:347:26: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
PWGLF/Tasks/Resonances/omega2012TrainingTable.cxx:560:24: warning: redundant explicit casting to the same type 'int64_t' (aka 'long') as the sub-expression, remove this casting [readability-redundant-casting]
PWGLF/Tasks/Resonances/omega2012TrainingTable.cxx:567:24: warning: redundant explicit casting to the same type 'int64_t' (aka 'long') as the sub-expression, remove this casting [readability-redundant-casting]
++ [[ 3 -gt 0 ]]
++ exit 1
--

Full log here.

@BongHwi
BongHwi enabled auto-merge (squash) October 23, 2026 13:42
@vkucera

vkucera commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

@BongHwi Please do not merge the PR with errors.

Mark MC-only callback parameters as maybe_unused and remove redundant
integer casts. Bind fixed-size daughter index arrays by reference.
Keep the persistent K1 feature column format and document its narrow
exception to modernize-avoid-c-arrays.
@BongHwi
BongHwi disabled auto-merge October 23, 2026 14:11
@BongHwi
BongHwi enabled auto-merge (squash) October 23, 2026 14:55
@BongHwi

BongHwi commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @vkucera , I clicked the merge before I found the error!
Now it is fine.

@vkucera

vkucera commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Thanks @vkucera , I clicked the merge before I found the error! Now it is fine.

Thanks @BongHwi !

@vkucera

vkucera commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

@BongHwi It seems you don't own any of the files so enabling auto-merge is quite counterproductive because it will need to be disabled by someone anyway.

@BongHwi

BongHwi commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

I get the concern, but these are actually classes I manage (if you meant the CODEOWNERS, no I don't have, of course). Auto-merge won’t push anything through unless CI passes and an approval is granted anyway, so it felt pretty safe.

I usually enable auto-merge because of the Asia time difference, it helps the PR get merged (if it is approved meanwhile) before midnight while I'm asleep so it makes the daily tag build.

Happy to leave it off though if that’s our convention!

@vkucera

vkucera commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

I get the concern, but these are actually classes I manage (if you meant the CODEOWNERS, no I don't have, of course). Auto-merge won’t push anything through unless CI passes and an approval is granted anyway, so it felt pretty safe.

I usually enable auto-merge because of the Asia time difference, it helps the PR get merged (if it is approved meanwhile) before midnight while I'm asleep so it makes the daily tag build.

Happy to leave it off though if that’s our convention!

I don't think you understand how auto-merge works. You are actually blocking the merging by enabling the auto-merge. Please read documentation in the contribution guidelines and the last line of the action output:

This check will run again when the PR's author disables and reenables auto-merge.

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

Development

Successfully merging this pull request may close these issues.

3 participants