[PWGHF] KFParticle based 3 prong decay reconstruction development #10610

lubynets · 2025-03-21T18:29:33Z

In this PR usage of KF Particle for 3 prong decay (in particular Lc->PKPi) reconstruction was developed as well as some changes needed for Lc lifetime measurement were implemented:

Application of topological and invariant mass constraints implemented.
Enabled application of cuts on Kf-based variables in the candidateSelectorLc task.
Added lifetime-relevant fields to the Mc particles table of the treeCreatorLcToPKPi task.
List of pid-related variables in the treeCreatorLcToPKPi task reduced to what is really needed according to the order of prongs in the candidate - PKPi or PiKP.

…SelectorLc

…pAll

PWGHF/Core/SelectorCuts.h

PWGHF/TableProducer/treeCreatorLcToPKPi.cxx

fgrosa

Good for me now, thanks @lubynets for reverting the PID variables!

fgrosa · 2025-04-22T08:30:35Z

Hi @lubynets, is this PR ready? Should we revert it to Ready for review? Thanks!

lubynets · 2025-04-22T08:32:23Z

Hi @fgrosa,
Yes, it's ready for review. Sorry for the silence from my side.

vkucera · 2025-04-22T09:34:34Z

@lubynets Please fix the issues.

lubynets · 2025-04-22T09:47:50Z

@lubynets Please fix the issues.

@vkucera, I noticed the "magic number" issues, but I could not invent a meaningful name for the variable 2, because I do not understand the physics case - why a 3-prong decay particle has only two daughters. Can you please briefly hint what it means, so I could create a proper alias for 2?

vkucera · 2025-04-22T09:52:50Z

PWGHF/TableProducer/treeCreatorLcToPKPi.cxx

+            const float x = candidate.xSecondaryVertex();
+            const float y = candidate.ySecondaryVertex();
+            const float z = candidate.zSecondaryVertex();
+            const float errX = candidate.kfXError();
+            const float errY = candidate.kfYError();
+            const float errZ = candidate.kfZError();
+            const float errPVX = candidate.kfXPVError();
+            const float errPVY = candidate.kfYPVError();
+            const float errPVZ = candidate.kfZPVError();
+            const float chi2primKaon = candidate.kfChi2PrimProng1();
+            const float dcaProtonPion = candidate.kfDcaProng0Prong2();
+            const float chi2GeoProtonPion = candidate.kfChi2GeoProng0Prong2();


The amount of code duplication is increasing with every addition. Please take a moment for a proper factorisation soon.

Ok, I will try to reduce code duplication between MC and data functions.
There is one more place where duplication takes place - the lite and full tables. Won't it be better if they will be replaced with lite and "additional", where lite will remain as is, and "additional" will include only the difference between full and lite?
Or due to compatibility with other tasks the existing division into lite and full tables should remain?

Hi @lubynets @vkucera if you agree, I would postpone it to a separated PR, since it would touch only one file while the current one is open since a while and it is implementing modifications to a few files that we would like to modify for a couple of central developments (see for event selection #10949, and another one regards the PID variables). If you confirm that it is ok for you, I would merge this PR. Thanks!

Hi @fgrosa (@vkucera cc),
From my side it's ok to prepare a separate PR dedicated to code duplication reduction (if Vit does not object).
I would also like to clarify whether my idea expressed above (replace "full" table with "additional / extended / etc") makes sense? Or should I contact those who is marked as code authors - Nicolo' and Luigi?

@lubynets thanks!
Regarding the names, while I understand the reasoning, I would keep full simply because it is the same for all channels and it's easier for newcomers to have similar names (or you change it for all the channels), but not a strong opinion ;-)

No, my idea is not about names change! (the name can be approved later)
The point is that there are two possibilities - to write Lite or Full table on user's request according to the Configurable flag. But the Lite is a subset of Full, so there are a lot of identical code lines, which we want to reduce.
My suggestion is to replace Full with Full* (the name can be a subject of discussion) which will contain the difference between Full and Lite - then there will be no code duplication. Then the Lite will be written always, by default, and Full* - only by user's request of one needs to save more information.
But I am not sure if it will be compatible with other tasks, and if it's OK to have two output tables instead of one.

I see, sorry for the misunderstanding! In principle it is OK to have 2 tables instead of one, however it is not very user friendly for local processing (since the tree creator is meant to produce a flat tree for analysis). If you use O2 or python (e.g. pandas) is easy to join two "tables", but otherwise not much, so I would still keep the lite and full so that it's easier for less expert users (but honestly speaking I also don't know how many users use the full version..)

I was not suggesting to do the factorisation in this PR, so I'm fine with the idea of a dedicated PR after the central features are merged.

vkucera · 2025-04-22T09:56:57Z

@lubynets Please fix the issues.

@vkucera, I noticed the "magic number" issues, but I could not invent a meaningful name for the variable 2, because I do not understand the physics case - why a 3-prong decay particle has only two daughters. Can you please briefly hint what it means, so I could create a proper alias for 2?

It's the number of daughters in the resonant decay channel.

constexpr std::size_t NDaughtersResonant{2u};

lubynets · 2025-04-22T10:30:35Z

@lubynets Please fix the issues.

Done

alibuild · 2025-04-23T09:55:43Z

Error while checking build/O2Physics/o2 for 1df1a8e at 2025-04-23 11:55:

## sw/BUILD/O2Physics-latest/log
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
Error in cling::AutoLoadingVisitor::InsertIntoAutoLoadingState:
/sw/SOURCES/O2Physics/10610-slc9_x86-64/0/PWGHF/TableProducer/candidateCreator3Prong.cxx:111:3: error: non-static data member 'NDaughtersResonant' declared 'constexpr'
/sw/SOURCES/O2Physics/10610-slc9_x86-64/0/PWGHF/TableProducer/candidateCreator3Prong.cxx:951:39: error: 'NDaughtersResonant' was not declared in this scope
/sw/SOURCES/O2Physics/10610-slc9_x86-64/0/PWGHF/TableProducer/candidateCreator3Prong.cxx:997:39: error: 'NDaughtersResonant' was not declared in this scope
ninja: build stopped: subcommand failed.

Full log here.

…iceO2Group#10610)

lubynets added 12 commits March 14, 2025 19:37

resolve conflict kfPV: apply mass and topo constraints

9111ce9

resolve conflict linter NBinsPt: implement KF-based cuts in candidate…

078b048

…SelectorLc

fix errors in prev. 2 commits

d517cf1

transport candidate back to decay vertex after topological constraint

57a1086

add lifetime-relevant fields to mc particles table

53da407

bugfix wrong type in _as<>

bdbc872

add N PV tracks for each candidate

70712a3

add some event info to kf cand table

380ba0a

make write-candidate condition more clear; enable BG scaling when kee…

78cf883

…pAll

add MC and error vtx pos to HfCandLcFullEvs

b15aaa9

rename some vars according to master version

27d31b7

stick pid sigma variables to a certain PKPi / PiKP hypothesis

030fdbd

lubynets requested review from NicoleBastid, alibuild, apalasciano, deepathoms, fcatalan92, fcolamar, fgrosa, hahassan7, jpxrk, mfaggin, mmazzilli, vkucera and zhangbiao-phy as code owners March 21, 2025 18:29

github-actions bot added the pwghf PWG-HF label Mar 21, 2025

github-actions bot changed the title ~~KFParticle based 3 prong decay reconstruction development~~ [PWGHF] KFParticle based 3 prong decay reconstruction development Mar 21, 2025

vkucera reviewed Mar 21, 2025

View reviewed changes

PWGHF/Core/SelectorCuts.h Outdated Show resolved Hide resolved

PWGHF/Core/SelectorCuts.h Outdated Show resolved Hide resolved

PWGHF/Core/SelectorCuts.h Outdated Show resolved Hide resolved

vkucera reviewed Mar 21, 2025

View reviewed changes

PWGHF/TableProducer/treeCreatorLcToPKPi.cxx Outdated Show resolved Hide resolved

lubynets requested a review from alcaliva as a code owner March 22, 2025 19:06

remove full::CollisionId from HFCANDLCKF table to allow merging

6c8ff35

lubynets requested review from fgrosa, mfaggin and vkucera April 3, 2025 12:45

fgrosa previously approved these changes Apr 3, 2025

View reviewed changes

vkucera reviewed Apr 22, 2025

View reviewed changes

replace magic number 2 with NDaughtersResonant

1df1a8e

lubynets dismissed fgrosa’s stale review via 1df1a8e April 22, 2025 10:13

fgrosa marked this pull request as ready for review April 23, 2025 08:53

bugfix of previous commit

9162b74

lubynets requested review from fgrosa and vkucera April 24, 2025 08:17

fgrosa approved these changes Apr 24, 2025

View reviewed changes

fgrosa merged commit b0a9d21 into AliceO2Group:master Apr 24, 2025
19 checks passed

lubynets deleted the KFdev branch April 24, 2025 14:24

ppoava pushed a commit to ppoava/O2Physics that referenced this pull request Apr 28, 2025

[PWGHF] KFParticle based 3 prong decay reconstruction development (Al…

6986e3e

…iceO2Group#10610)

bghanley1995 pushed a commit to bghanley1995/O2Physics that referenced this pull request May 5, 2025

[PWGHF] KFParticle based 3 prong decay reconstruction development (Al…

23766bb

…iceO2Group#10610)

jinhyunni pushed a commit to jinhyunni/O2Physics that referenced this pull request May 11, 2025

[PWGHF] KFParticle based 3 prong decay reconstruction development (Al…

1c73185

…iceO2Group#10610)

jinhyunni pushed a commit to jinhyunni/O2Physics that referenced this pull request May 11, 2025

[PWGHF] KFParticle based 3 prong decay reconstruction development (Al…

eab08dd

…iceO2Group#10610)

prottayCMT pushed a commit to prottayCMT/O2Physics2024 that referenced this pull request May 17, 2025

[PWGHF] KFParticle based 3 prong decay reconstruction development (Al…

00d4813

…iceO2Group#10610)

ariedel-cern pushed a commit to ariedel-cern/O2Physics that referenced this pull request May 23, 2025

[PWGHF] KFParticle based 3 prong decay reconstruction development (Al…

5845b4a

…iceO2Group#10610)

smaff92 pushed a commit to smaff92/O2Physics that referenced this pull request Jun 17, 2025

[PWGHF] KFParticle based 3 prong decay reconstruction development (Al…

fc670ef

…iceO2Group#10610)

alibuild pushed a commit to alibuild/O2Physics that referenced this pull request Aug 11, 2025

[PWGHF] KFParticle based 3 prong decay reconstruction development (Al…

f768d8c

…iceO2Group#10610)

[PWGHF] KFParticle based 3 prong decay reconstruction development #10610

[PWGHF] KFParticle based 3 prong decay reconstruction development #10610

Uh oh!

Conversation

lubynets commented Mar 21, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fgrosa left a comment

Choose a reason for hiding this comment

Uh oh!

fgrosa commented Apr 22, 2025

Uh oh!

lubynets commented Apr 22, 2025

Uh oh!

vkucera commented Apr 22, 2025

Uh oh!

lubynets commented Apr 22, 2025

Uh oh!

vkucera Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

lubynets Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

fgrosa Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

lubynets Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

fgrosa Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lubynets Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

fgrosa Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

vkucera Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

vkucera commented Apr 22, 2025

Uh oh!

lubynets commented Apr 22, 2025

Uh oh!

alibuild commented Apr 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

fgrosa Apr 24, 2025 •

edited

Loading