Skip to content

Conversation

@abmodak
Copy link
Collaborator

@abmodak abmodak commented Aug 4, 2025

  1. Fix the initialisation of fv0det geometry pointer
  2. Add addHistos function instead of using addClone

1) Fix the initialisation of `fv0det` geometry pointer 
2) Add `addHistos` function instead of using `addClone`
@github-actions
Copy link

github-actions bot commented Aug 4, 2025

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

@github-actions github-actions bot changed the title Fix bug in FV0 eta calculation [PWGCF] Fix bug in FV0 eta calculation Aug 4, 2025

static constexpr std::string_view kCorrType[] = {"Ft0aGlobal/", "Ft0cGlobal/", "Fv0Global/", "MftGlobal/", "Fv0Mft/"};
static constexpr std::string_view kEvntType[] = {"SE/", "ME/"};

Copy link
Collaborator

@victor-gonzalez victor-gonzalez Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for considering my suggestions!

I have an additional one for security and error avoidance in case you want to also consider it

I would suggest not trusting on remembering the meaning of 0 or 3 or of the position in the template parameters
Two additional enums can be defined

enum KindOfMixing {
  kSE,
  kME
};

enum KindOfMultiplicity {
  kFT0AGLOBAL,
  kFT0CGLOBAL,
  kFV0GLOBAL,
  kMFTGLOBAL,
  kFV0MFT
};

and the functions can be templated with this types, for instance

  template <KindOfMultiplicity corrType, KindOfMixing evntType>
  void addHistos(...)

Now if always the named constants are used the compiler will advice if there is a misuse at any point

addHistos<kFT0GLOBAL, kSE>(...)

will compile correctly but

addHistos<kSE, kFT0GLOBAL>(...)

will give an error

The chosen names are, of course, representative

Copy link
Collaborator

@victor-gonzalez victor-gonzalez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a look at my additional comment in case you want to consider it for future iterations

@victor-gonzalez victor-gonzalez enabled auto-merge (squash) August 4, 2025 19:46
@victor-gonzalez victor-gonzalez merged commit b5d6a9c into AliceO2Group:master Aug 5, 2025
14 checks passed
@abmodak abmodak deleted the update-longrange branch August 11, 2025 03:40
jpxrk pushed a commit to jpxrk/O2Physics that referenced this pull request Aug 12, 2025
jloemker pushed a commit to jloemker/O2Physics that referenced this pull request Nov 11, 2025
alibuild pushed a commit to alibuild/O2Physics that referenced this pull request Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants