Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PWGCF,PWGDQ] Weight merging #9627

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

EmilGorm
Copy link
Contributor

@EmilGorm EmilGorm commented Jan 30, 2025

Attempt to fix Hyperloop-only merge issue for dynamically added GFWWeights

  • Added Merge function to GFWWeightsList
  • Added GFWWeightsList as output in flowGenericFramework task, replaces GFWWeights stored in TList
  • Modified input corrections to read GFWWeightsList over Tlist
  • Both GFWWeightsList and GFWWeights are fully backwards compatible aside from the change to camelCase

O2 linter:

  • A persistent request to change to camelCase for function names clashes with ROOTs naming convention for the Merge function
  • Otherwise I have followed the suggestions, but as multiple analysis tasks use the GFWWeights, I had to update the calls to GFWWeights to reflect the O2 Linter camelCase suggestions
  • Remaining O2 Linter suggestions will be the individual users' responsibility (Similar for remaining Mega Linter)

@github-actions github-actions bot changed the title Weight merging [PWGCF,PWGDQ] Weight merging Jan 30, 2025
@alibuild
Copy link
Collaborator

Error while checking build/O2Physics/o2 for a9cc77a at 2025-01-30 17:31:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowRunbyRun.cxx:322:21: error: 'class GFWWeights' has no member named 'Fill'; did you mean 'fill'?
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowSP.cxx:169:17: error: 'class GFWWeights' has no member named 'SetPtBins'; did you mean 'setPtBins'?
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowSP.cxx:170:17: error: 'class GFWWeights' has no member named 'Init'; did you mean 'init'?
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowSP.cxx:172:20: error: 'class GFWWeights' has no member named 'SetPtBins'; did you mean 'setPtBins'?
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowSP.cxx:173:20: error: 'class GFWWeights' has no member named 'Init'; did you mean 'init'?
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowSP.cxx:175:20: error: 'class GFWWeights' has no member named 'SetPtBins'; did you mean 'setPtBins'?
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowSP.cxx:176:20: error: 'class GFWWeights' has no member named 'Init'; did you mean 'init'?
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowSP.cxx:375:42: error: 'class GFWWeights' has no member named 'GetNUA'; did you mean 'getNUA'?
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowSP.cxx:712:21: error: 'class GFWWeights' has no member named 'Fill'; did you mean 'fill'?
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowSP.cxx:715:26: error: 'class GFWWeights' has no member named 'Fill'; did you mean 'fill'?
/sw/SOURCES/O2Physics/9627-slc9_x86-64/0/PWGCF/Flow/Tasks/flowSP.cxx:718:26: error: 'class GFWWeights' has no member named 'Fill'; did you mean 'fill'?
ninja: build stopped: subcommand failed.

Full log here.

@victor-gonzalez
Copy link
Collaborator

victor-gonzalez commented Jan 30, 2025

Hi Emil, thanks for doing this!!!
Be careful
One linter recommendation is really mandatory because it actually has dangerous effects if not followed
In fact, the code in FlowGFWPbPb.cxx is working in a not expected way
I'm referring to incorporate std:: before abs
The behavior of abs, alone, has changed a while ago in the grid machines. Now it always round the argument to the previous integer, which is not expected. With the std:: prefix it works as expected, integer when the argument is integer and float when the argument is float

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