[PWGCF] Implementation of Phi and K0Short by gitchrishub-oss · Pull Request #12738 · AliceO2Group/O2Physics

gitchrishub-oss · 2025-08-25T19:55:48Z

Resonance selection via reconstruction of the invariant mass has been implemented in the producer Task.
Several files were modified/added in order to support this implementation.
Efforts were made to comply with linter and formatting suggestions; however, some warnings persist due existing files that were not modified.
@AntonRiedel
@gmantzar

github-actions · 2025-08-25T19:56:03Z

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

victor-gonzalez · 2025-08-25T20:11:53Z

Please, keep it as draft until approaching its final approval
Thanks!

victor-gonzalez · 2025-08-26T19:18:24Z

Please, keep it as draft for the time being
Reviewers will be able to comment and suggest modifications and global CI resources will not be wasted
Thanks!!!

gitchrishub-oss · 2025-09-12T14:26:48Z

@victor-gonzalez I have a question regarding the security approval required for the build checks.
What kind of approval is needed, and from whom should I request it?

lauraser · 2025-09-17T07:44:28Z

PWGCF/DataModel/FemtoDerived.h

+static constexpr std::string_view ParticleTypeName[kNParticleTypes] = {"Track", "V0", "V0Child", "Cascade", "CascadeV0", "CascadeV0Child", "CascadeBachelor", "CharmHadron", "Reso", "ResoChild", "ResoPosdaughTPC_NegdaughTPC", "ResoPosdaughTPC_NegdaughTOF", "ResoPosdaughTOF_NegdaughTPC", "ResoPosdaughTOF_NegdaughTOF"}; //! Naming of the different particle types
+// static constexpr std::string_view TempFitVarName[kNParticleTypes] = {"/hDCAxy", "/hCPA", "/hDCAxy", "/hCPA", "/hCPA", "/hDCAxy", "/hDCAxy", "/hCPA"};
+
+static constexpr std::string_view TempFitVarName[kNParticleTypes] = {"/hDCAxy", "/hCPA", "/hCPA", "/hCPA", "/hCPA", "/hDCAxy", "/hDCAxy", "/hCPA", "/hDCAxy", "/hDCAxy", "/hDCAxy", "/hDCAxy", "/hDCAxy", "/hDCAxy"}; // change later!! check for DCAXY for RESO!!


Why did this list change from the previous? To be backwards compatible, they should be identical for the first 8 entries, no?

The correct order inside TempFitVarName was restored

lauraser · 2025-09-17T09:06:01Z

I do not see any more problems. @victor-gonzalez , maybe you could have a last look to check if I didn't miss anything?

victor-gonzalez · 2025-09-17T10:13:04Z

PWGCF/FemtoDream/Core/femtoDreamTrackSelection.h

  /// \param track Track
  /// \return Whether the most open combination of all selection criteria is fulfilled
-  template <typename T>
+  template <class T>


Strange that this needed to be changed just now. In principle typename is larger than class embracing it
I guess if local compilation succeeded there should not be any issue with it

AFAIK, the typename and class keywords are equivalent in the context of template arguments. What do you mean by "typename is larger than class"?

I thought that typename can be any type as int for example while class can only be a class or a struct
Larger in the sense that the amount of cases that address is larger

victor-gonzalez · 2025-09-17T10:17:21Z

PWGCF/FemtoDream/Core/femtoDreamUtils.h

+    case o2::constants::physics::Pdg::kK0Star892:
+      mass = o2::constants::physics::MassK0Star892;
+      break;
+    case 310: /// K0Short is not implemented in o2::physics::constants::Pdg


The recommendation is to extend the definitions with the ones in TPDGCode.h

kK0Short already exists in TPDGCode.h.

victor-gonzalez · 2025-09-17T10:19:32Z

PWGCF/FemtoDream/Core/femtoDreamV0SelectionK0Short.h

+#include <string>
+#include <vector>
+
+namespace o2::analysis::femtoDream // o2-linter: disable=name/namespace (Previously defined namespace)


Don't disable it for the linter. I guess this is a false positive we have to report to @vkucera

This is a true positive.

@vkucera I don't understand this now then
He is trying to incorporate declarations to the femtoDream namespace
And the usage of that namespace is open and then closed for such declarations once done
Cannot this be done on header files? If it can, what should be the procedure?

There is nothing wrong with the code itself. The name/namespace test complains about the femtoDream name not following the snake_case convention for namespaces.

Ok! Wrong approach from my side!
Then this is going to stay because it affects the full framework which was born before linter recommendations

victor-gonzalez · 2025-09-17T10:22:47Z

PWGCF/FemtoDream/TableProducer/CMakeLists.txt

+          PUBLIC_LINK_LIBRARIES O2::Framework O2Physics::AnalysisCore O2Physics::EventFilteringUtils
+          COMPONENT_NAME Analysis)
+
 o2physics_add_dpl_workflow(femtodream-producer-reduced


The linter should be complaining in here
If you want to keep the name of the executable you have to change the name of the source file or the other way around. Follow the recommendation from the linter. You can execute the linter locally with the instructions given at the end of the linter log

It does complain.

PWGCF/FemtoDream/TableProducer/CMakeLists.txt:17: error: Workflow name femtodream-producer-reso does not match its file name femtoDreamProducerTaskReso.cxx. (Matches femtodreamProducerReso.cxx.) [name/o2-workflow]

victor-gonzalez

Minor comments but better addressing them now before starting

vkucera · 2025-09-17T12:42:06Z

PWGCF/FemtoDream/Core/femtoDreamV0SelectionK0Short.h

+  static constexpr int kNv0Selection = 9;
+
+  static constexpr std::string_view kSelectionNames[kNv0Selection] = {
+    "Sign", "PtMin", "PtMax", "EtaMax", "DCAdaughMax", "CPAMin",
+    "TranRadMin", "TranRadMax", "DecVecMax"}; ///< Name of the different
+                                              ///< selections
+
+  static constexpr femtoDreamSelection::SelectionType
+    kSelectionTypes[kNv0Selection]{
+      femtoDreamSelection::kEqual,
+      femtoDreamSelection::kLowerLimit,
+      femtoDreamSelection::kUpperLimit,
+      femtoDreamSelection::kUpperLimit,
+      femtoDreamSelection::kUpperLimit,
+      femtoDreamSelection::kLowerLimit,
+      femtoDreamSelection::kLowerLimit,
+      femtoDreamSelection::kUpperLimit,
+      femtoDreamSelection::kUpperLimit}; ///< Map to match a variable with


Please stick to the naming conventions and don't use the k prefix by default. It makes expressions like kSelectionNames[kNv0Selection] quite confusing, since the constants starting with k are usually reserved for sizes of static arrays.

vkucera · 2025-09-17T12:44:13Z

I do not see any more problems. @victor-gonzalez , maybe you could have a last look to check if I didn't miss anything?

I see many. :-)

the formatting error in FemtoDerived.h was removed. In femtoDreamUtils.h was the 310 replaced by kK0Short (as in TPDGCode.h) and in KV0SelectionK0Short.h were the k's replaced by m's.

victor-gonzalez · 2025-09-18T08:41:44Z

We don't expect the linter errors of the existing files being fixed when introducing minor changes
But the new files should enter in the repo without linter errors
There are still the below errors that can be fixed
Once this is done the PR will be set to ready and approved from my side
@lauraser, do you agree with that?

PWGCF/FemtoDream/Core/femtoDreamV0SelectionK0Short.h:254: error: Use UpperCamelCase for names of constexpr constants. Names of special constants may be prefixed with "k". [name/constexpr-constant]
Error: Use UpperCamelCase for names of constexpr constants. Names of special constants may be prefixed with "k".
PWGCF/FemtoDream/Core/femtoDreamV0SelectionK0Short.h:272: error: Use UpperCamelCase for names of constexpr constants. Names of special constants may be prefixed with "k". [name/constexpr-constant]
Error: Use UpperCamelCase for names of constexpr constants. Names of special constants may be prefixed with "k".

PWGCF/FemtoDream/TableProducer/femtoDreamProducerTaskReso.cxx:307: error: Use constant references for non-modified iterators in range-based for loops. [const-ref-in-for-loop]
Error: Use constant references for non-modified iterators in range-based for loops.
PWGCF/FemtoDream/TableProducer/femtoDreamProducerTaskReso.cxx:592: error: Avoid magic numbers in expressions. Assign the value to a clearly named variable or constant. [magic-number]
Error: Avoid magic numbers in expressions. Assign the value to a clearly named variable or constant.
PWGCF/FemtoDream/TableProducer/femtoDreamProducerTaskReso.cxx:601: error: Avoid magic numbers in expressions. Assign the value to a clearly named variable or constant. [magic-number]
Error: Avoid magic numbers in expressions. Assign the value to a clearly named variable or constant.
PWGCF/FemtoDream/TableProducer/femtoDreamProducerTaskReso.cxx:430: error: Use lowerCamelCase for names of functions and variables. [name/function-variable]
Error: Use lowerCamelCase for names of functions and variables.
PWGCF/FemtoDream/TableProducer/femtoDreamProducerTaskReso.cxx:128: error: Use lowerCamelCase for names of configurables and use the same name for the struct member as for the JSON string. (Declare the type and names on the same line.) [name/configurable]
Error: Use lowerCamelCase for names of configurables and use the same name for the struct member as for the JSON string. (Declare the type and names on the same line.)

gitchrishub-oss · 2025-09-18T09:06:51Z

i have a question regarding the k prefix linter errors which were raised in V0SelectionK0Short.h. These are the linter errors is solved by renaming mSelectionNames etc to kSelectionNames. In one of the latest commit messages i was asked to change them due to possible confusion. Should change the rpefixes back to k or leave them as is?

vkucera · 2025-09-18T11:38:36Z

i have a question regarding the k prefix linter errors which were raised in V0SelectionK0Short.h. These are the linter errors is solved by renaming mSelectionNames etc to kSelectionNames. In one of the latest commit messages i was asked to change them due to possible confusion. Should change the rpefixes back to k or leave them as is?

I think O2 linter gave you clear instructions what to do:

Use UpperCamelCase for names of constexpr constants.

linter errrs were adressed in femtoDreamPorducerTaskReso.cxx and V0SelectionK0Short.h

PWGCF/FemtoDream/Core/femtoDreamV0SelectionK0Short.h

vkucera · 2025-09-18T13:53:24Z

PWGCF/FemtoDream/TableProducer/femtoDreamProducerTaskReso.cxx

+      constexpr int kProcessDirectMother = 4;
+      constexpr int kProcessInelasticHadronic = 23;
+      constexpr int kGenStatusTransport = -1;


Do not redefine existing constants. Use the actual enumerated values from TMCProcess.

victor-gonzalez

Thanks!!!

gitchrishub-oss · 2025-09-19T12:05:43Z

Thank you very much for you help!

Co-authored-by: Christopher Klumm <ge48xun@nidoking.ktas.ph.tum.de> Co-authored-by: ALICE Action Bot <alibuild@cern.ch>

gitchrishub-oss requested review from alibuild, ariedel-cern, glromane, jgrosseo, lauraser, lgraczykCern, otonvd, prchakra, saganatt, shouqiye, victor-gonzalez and zchochul as code owners August 25, 2025 19:55

github-actions bot added pwgcf datamodel labels Aug 25, 2025

alibuild mentioned this pull request Aug 25, 2025

Please consider the following formatting changes to #12738 gitchrishub-oss/O2Physics#1

Closed

victor-gonzalez marked this pull request as draft August 25, 2025 20:10

gitchrishub-oss force-pushed the reso branch from 40b2b6d to d6daaad Compare August 26, 2025 11:40

gitchrishub-oss marked this pull request as ready for review August 26, 2025 16:31

victor-gonzalez marked this pull request as draft August 26, 2025 19:16

gitchrishub-oss force-pushed the reso branch from 1ce282e to dbb961e Compare September 11, 2025 13:39

alibuild mentioned this pull request Sep 11, 2025

Please consider the following formatting changes to #12738 gitchrishub-oss/O2Physics#2

Closed

further file changes

d05d5f9

gitchrishub-oss force-pushed the reso branch from dbb961e to d05d5f9 Compare September 11, 2025 13:57

github-actions bot removed the datamodel label Sep 11, 2025

Christopher Klumm added 2 commits September 12, 2025 12:26

formatting changes

2b0f5ed

further formatting changes

dad3ee8

updated femtoDreamUtils.h

21bab1f

lauraser reviewed Sep 17, 2025

View reviewed changes

change in Femtoderived.h

afba04a

The correct order inside TempFitVarName was restored

alibuild mentioned this pull request Sep 17, 2025

Please consider the following formatting changes to #12738 gitchrishub-oss/O2Physics#4

Closed

victor-gonzalez reviewed Sep 17, 2025

View reviewed changes

vkucera reviewed Sep 17, 2025

View reviewed changes

Christopher Klumm added 3 commits September 17, 2025 15:53

implementation of suggested changes

4acc828

the formatting error in FemtoDerived.h was removed. In femtoDreamUtils.h was the 310 replaced by kK0Short (as in TPDGCode.h) and in KV0SelectionK0Short.h were the k's replaced by m's.

Change in copyright header

c483628

Two times the charm

223a119

adressing linter errors

e33dbc6

linter errrs were adressed in femtoDreamPorducerTaskReso.cxx and V0SelectionK0Short.h

vkucera reviewed Sep 18, 2025

View reviewed changes

further changes

9a3b757

victor-gonzalez marked this pull request as ready for review September 18, 2025 16:21

victor-gonzalez approved these changes Sep 18, 2025

View reviewed changes

victor-gonzalez merged commit ef72c9b into AliceO2Group:master Sep 19, 2025
12 of 13 checks passed

jmunozme pushed a commit to jmunozme/O2Physics that referenced this pull request Oct 3, 2025

[PWGCF] Implementation of Phi and K0Short (AliceO2Group#12738)

85ee733

Co-authored-by: Christopher Klumm <ge48xun@nidoking.ktas.ph.tum.de> Co-authored-by: ALICE Action Bot <alibuild@cern.ch>

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

[PWGCF] Implementation of Phi and K0Short (AliceO2Group#12738)

c324449

Co-authored-by: Christopher Klumm <ge48xun@nidoking.ktas.ph.tum.de> Co-authored-by: ALICE Action Bot <alibuild@cern.ch>

ThePhDane pushed a commit to ThePhDane/O2Physics that referenced this pull request Nov 3, 2025

[PWGCF] Implementation of Phi and K0Short (AliceO2Group#12738)

c06ce27

Co-authored-by: Christopher Klumm <ge48xun@nidoking.ktas.ph.tum.de> Co-authored-by: ALICE Action Bot <alibuild@cern.ch>

jloemker pushed a commit to jloemker/O2Physics that referenced this pull request Nov 11, 2025

[PWGCF] Implementation of Phi and K0Short (AliceO2Group#12738)

5528247

Co-authored-by: Christopher Klumm <ge48xun@nidoking.ktas.ph.tum.de> Co-authored-by: ALICE Action Bot <alibuild@cern.ch>

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

[PWGCF] Implementation of Phi and K0Short (AliceO2Group#12738)

81ac249

Co-authored-by: Christopher Klumm <ge48xun@nidoking.ktas.ph.tum.de> Co-authored-by: ALICE Action Bot <alibuild@cern.ch>

Conversation

gitchrishub-oss commented Aug 25, 2025

Uh oh!

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

Uh oh!

Uh oh!

victor-gonzalez commented Aug 25, 2025

Uh oh!

victor-gonzalez commented Aug 26, 2025

Uh oh!

gitchrishub-oss commented Sep 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lauraser commented Sep 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

victor-gonzalez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vkucera commented Sep 17, 2025

Uh oh!

victor-gonzalez commented Sep 18, 2025

Uh oh!

gitchrishub-oss commented Sep 18, 2025

Uh oh!

vkucera commented Sep 18, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

victor-gonzalez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gitchrishub-oss commented Sep 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Aug 25, 2025 •

edited

Loading