PWGHF: Add XicPlus to Xi Pi Pi workflow by pstahlhu · Pull Request #6604 · AliceO2Group/O2Physics

pstahlhu · 2024-06-20T16:49:31Z

First version of XicPlus to Xi Pi Pi workflow
@creetz16

Please consider the following formatting changes to AliceO2Group#6604

mfaggin

thanks a lot for the developments, I have some commets/suggestions

mfaggin · 2024-06-21T15:40:11Z

+                  full::XiId,
+                  full::Pi0Id,
+                  full::Pi1Id,


Just a warning: if you goal is to create self-contained derived data then I think there are no problems. But beware that in case you just want slim derived data (e.g. a tree to train your ML model) then with INDEX_COLUMNS you will have issues with unbound indices in Hyperloop.

My goal was to compare different configurations (e.g. KF vs. DCA, KFs with different combination of constraints) in order to check how to optimize the reconstruction of the XicPlus. I added the indices to ensure that I would then compare the identical sample of XicPlus candidates.

Do you thin this would work? Or are there also problems with unbound indices?

Is this a problem also for running on locally stored derived data?

Can I store the indices or do something similar without this problem of unbound indices? If yes, how?

If you want to be flexible and have the possibility to both include or esclude the index columns, maybe you can do a separate table only with these columns, to be filled only on demand at runtime (i.e. ruling it with a dedicated Configurable)

alibuild · 2024-06-21T17:45:00Z

Error while checking build/O2Physics/o2 for 0054da2 at 2024-06-21 19:44:

No log files found

Full log here.

pstahlhu · 2024-06-23T18:00:55Z

Hi @vkucera and @mfaggin,
thank you very much for your comments!! I implemented most of your requested changes now.
For some I have some comments or questions myself, as you will see above...

vkucera · 2024-06-24T07:57:54Z

@pstahlhu , thanks for the updates. Please do not mark suggestions as resolved. The reviewers have to check their implementation and only then they can approve that the conversation has been resolved.

pstahlhu · 2024-06-24T11:01:49Z

Hi @vkucera, thanks for the additional comments, I will update the PR later.
And sorry for marking the previous suggestions as resolved, I wanted to indicate which suggestions I implemented, but of course it makes much more sense that you mark them as resolved once you are happy with them.

…utput tables, remove unused variables

…rack index skimming

…eator and task

… generated particles

…des for PR

pstahlhu · 2024-07-05T10:23:19Z

Hi @vkucera and @mfaggin, I have implemented your comments and updated the PR once again. Could you have another look? Many thanks in advance!!

vkucera · 2024-07-05T11:20:39Z

Hi @vkucera and @mfaggin, I have implemented your comments and updated the PR once again. Could you have another look? Many thanks in advance!!

Thanks @pstahlhu , I will have a look, but please do not force-push your branch when your PR is under review. It overwrites the commit history. See more details in the documentation.

vkucera · 2024-07-05T12:16:48Z

Hi @vkucera and @mfaggin, I have implemented your comments and updated the PR once again. Could you have another look? Many thanks in advance!!

Thanks @pstahlhu , I will have a look, but please do not force-push your branch when your PR is under review. It overwrites the commit history. See more details in the documentation.

The implementation of comments looks fine to me, thanks. I just have a few remaining comments. Then it is ready for merging for me.

pstahlhu · 2024-07-05T12:55:40Z

Hi @vkucera and @mfaggin, I have implemented your comments and updated the PR once again. Could you have another look? Many thanks in advance!!

Thanks @pstahlhu , I will have a look, but please do not force-push your branch when your PR is under review. It overwrites the commit history. See more details in the documentation.

The implementation of comments looks fine to me, thanks. I just have a few remaining comments. Then it is ready for merging for me.

@vkucera thanks for your review. I also implemented your remaining comments now.
Sorry for any inconvenience regarding the force-push, I will avoid it in future.

pstahlhu requested review from NicoleBastid, alibuild, deepathoms, fcatalan92, fcolamar, fgrosa, hahassan7, jpxrk, mfaggin, mmazzilli, nzardosh and vkucera as code owners June 20, 2024 16:49

alibuild mentioned this pull request Jun 20, 2024

Please consider the following formatting changes to #6604 pstahlhu/O2Physics#8

Merged

pstahlhu added a commit to pstahlhu/O2Physics that referenced this pull request Jun 20, 2024

Merge pull request #8 from alibuild/alibot-cleanup-6604

0006437

Please consider the following formatting changes to AliceO2Group#6604

vkucera requested changes Jun 21, 2024

View reviewed changes

mfaggin requested changes Jun 21, 2024

View reviewed changes

pstahlhu force-pushed the master branch from 0054da2 to 7a23931 Compare June 23, 2024 16:55

alibuild mentioned this pull request Jun 23, 2024

Please consider the following formatting changes to #6604 pstahlhu/O2Physics#9

Closed

vkucera reviewed Jun 24, 2024

View reviewed changes

Comment thread PWGHF/TableProducer/candidateCreatorXicToXiPiPi.cxx Outdated

vkucera reviewed Jun 24, 2024

View reviewed changes

Comment thread PWGHF/TableProducer/treeCreatorXicToXiPiPi.cxx Outdated

vkucera reviewed Jun 24, 2024

View reviewed changes

Comment thread PWGHF/TableProducer/candidateCreatorXicToXiPiPi.cxx Outdated

pstahlhu requested review from abylinkin, ddobrigk, maoyx, mfasDa and pbuehler as code owners June 26, 2024 14:43

pstahlhu and others added 15 commits July 5, 2024 11:33

Preparation for PR: Fix naming conventions, include KF variables in o…

cf115cd

…utput tables, remove unused variables

Fix kfXicPlus creation

3ab639d

Add Xi resonance to MC matching

0afa3ef

Restructure candidate creator: Remove PV refit, add KF process with t…

de5ce67

…rack index skimming

Add mass of Xi-pi pairs, correct DecayType flag conditions in tree cr…

b92138b

…eator and task

Remove ML selection + correct PID selection

4525402

Remove unused ML configurables from candidate selector

8ec55bb

Remove hfFlag, fix resonance matching + fix filling of histograms for…

21a9e8a

… generated particles

Remove process function independent of HfTrackIndexSkimCreatorLfCasca…

883acd5

…des for PR

Please consider the following formatting changes

7f4adb7

Fix MegaLinter error

0b55f5f

Add requested changes to PR

d48a2d6

Fix formatting

faad520

Implement requested changes in candidate and tree creator

62b34be

Tree creator: Move index columns to separate table

2675309

pstahlhu force-pushed the master branch from 0e569b5 to 2675309 Compare July 5, 2024 10:18

mfaggin previously approved these changes Jul 5, 2024

View reviewed changes

vkucera reviewed Jul 5, 2024

View reviewed changes

Comment thread PWGHF/DataModel/CandidateReconstructionTables.h Outdated

vkucera reviewed Jul 5, 2024

View reviewed changes

Comment thread PWGHF/TableProducer/candidateCreatorXicToXiPiPi.cxx Outdated

vkucera reviewed Jul 5, 2024

View reviewed changes

Comment thread PWGHF/TableProducer/candidateCreatorXicToXiPiPi.cxx Outdated

vkucera reviewed Jul 5, 2024

View reviewed changes

Comment thread PWGHF/TableProducer/candidateSelectorXicToXiPiPi.cxx Outdated

Implement requested changes from review

949171d

pstahlhu dismissed mfaggin’s stale review via 949171d July 5, 2024 12:45

vkucera approved these changes Jul 5, 2024

View reviewed changes

vkucera merged commit 76a6594 into AliceO2Group:master Jul 5, 2024

echizzal pushed a commit to echizzal/O2Physics that referenced this pull request Jul 18, 2024

PWGHF: Add XicPlus to Xi Pi Pi workflow (AliceO2Group#6604)

9f83435

Uh oh!

Conversation

pstahlhu commented Jun 20, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mfaggin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mfaggin Jun 21, 2024

Choose a reason for hiding this comment

Uh oh!

pstahlhu Jun 23, 2024

Choose a reason for hiding this comment

Uh oh!

mfaggin Jul 1, 2024

Choose a reason for hiding this comment

Uh oh!

alibuild commented Jun 21, 2024

Uh oh!

pstahlhu commented Jun 23, 2024

Uh oh!

vkucera commented Jun 24, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pstahlhu commented Jun 24, 2024

Uh oh!

pstahlhu commented Jul 5, 2024

Uh oh!

vkucera commented Jul 5, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vkucera commented Jul 5, 2024

Uh oh!

pstahlhu commented Jul 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants