Skip to content

[PWGHF] Adjust the structure of the Lc to pkpi task - #8210

Merged
vkucera merged 38 commits into
AliceO2Group:masterfrom
zhangbiao-phy:master
Nov 3, 2024
Merged

[PWGHF] Adjust the structure of the Lc to pkpi task#8210
vkucera merged 38 commits into
AliceO2Group:masterfrom
zhangbiao-phy:master

Conversation

@zhangbiao-phy

@zhangbiao-phyzhangbiao-phy commented Oct 29, 2024

Copy link
Copy Markdown
Collaborator
  • Fixed the default setting for the processes (there are 3 process functions be enabled simultaneously before)
  • Adjust the structure to analyze data and mc per collision, reference as taskDs.cxx
  • Add helper functions to make the structure clean (fillHistosMcGen, fillHistosMcRec, fillHistosData for the histogram and THnSparse filling)

@mfagginmfaggin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks @zhangbiao-phy , please consider my comments.
On top of them: have you compared the output with the new version of the code vs. the old one? Are they the same?

Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
@zhangbiao-phy

Copy link
Copy Markdown
CollaboratorAuthor

thanks @zhangbiao-phy , please consider my comments. On top of them: have you compared the output with the new version of the code vs. the old one? Are they the same?

Hi @mfaggin, yes, I have tested the Mc data, the histogram of the rec is the same for new and old. Then there is an issue introduced by my last commit for the generated particle loop. It can be fixed by run it per collision with this PR.

zhangbiao-phyand others added 6 commits October 30, 2024 09:56
Co-authored-by: Mattia Faggin <mattia.faggin@cern.ch>
Co-authored-by: Mattia Faggin <mattia.faggin@cern.ch>
Co-authored-by: Mattia Faggin <mattia.faggin@cern.ch>
Co-authored-by: Mattia Faggin <mattia.faggin@cern.ch>
Co-authored-by: Mattia Faggin <mattia.faggin@cern.ch>
mfaggin
mfaggin previously approved these changes Oct 30, 2024

@mfagginmfaggin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks a lot @zhangbiao-phy , approved

@mfaggin
mfaggin enabled auto-merge (squash) October 30, 2024 09:08
@alibuild

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/o2 for 772d2c7 at 2024-10-30 10:37:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/8210-slc7_x86-64/0/PWGHF/D2H/Tasks/taskLc.cxx:757:38: error: 'collision' was not declared in this scope; did you mean 'o2::aod::collision'?
/sw/SOURCES/O2Physics/8210-slc7_x86-64/0/PWGHF/D2H/Tasks/taskLc.cxx:767:37: error: 'collision' was not declared in this scope; did you mean 'o2::aod::collision'?
/sw/SOURCES/O2Physics/8210-slc7_x86-64/0/PWGHF/D2H/Tasks/taskLc.cxx:777:38: error: 'collision' was not declared in this scope; did you mean 'o2::aod::collision'?
/sw/SOURCES/O2Physics/8210-slc7_x86-64/0/PWGHF/D2H/Tasks/taskLc.cxx:797:38: error: 'collision' was not declared in this scope; did you mean 'o2::aod::collision'?
/sw/SOURCES/O2Physics/8210-slc7_x86-64/0/PWGHF/D2H/Tasks/taskLc.cxx:807:37: error: 'collision' was not declared in this scope; did you mean 'o2::aod::collision'?
ninja: build stopped: subcommand failed.

Full log here.

mfaggin
mfaggin previously approved these changes Oct 30, 2024
@vkucera
vkucera disabled auto-merge October 30, 2024 10:10
zhangbiao-phyand others added 3 commits October 31, 2024 16:21
Co-authored-by: Vít Kučera <vit.kucera@cern.ch>
Co-authored-by: Vít Kučera <vit.kucera@cern.ch>
Please consider the following formatting changes to AliceO2Group#8210
@zhangbiao-phy

zhangbiao-phy commented Nov 1, 2024

Copy link
Copy Markdown
CollaboratorAuthor

Hi @vkucera, I have commited the changes from your last comment, Could you please have a look? Thanks a lot!

Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskLc.cxx
Comment threadPWGHF/D2H/Tasks/taskLc.cxx
Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
@vkucera

Copy link
Copy Markdown
Collaborator

Fix the cpplint error

updated based on Vit's suggestions
Co-authored-by: Vít Kučera <vit.kucera@cern.ch>
@zhangbiao-phy

Copy link
Copy Markdown
CollaboratorAuthor

cpplint

@vkucera, I'm not sure why this error is occurring. I could add #include <vector> in the task, but I haven't seen it needed in other HF tasks. Do you have any idea why this might be happening?

@vkucera

Copy link
Copy Markdown
Collaborator

cpplint

@vkucera, I'm not sure why this error is occurring. I could add #include <vector> in the task, but I haven't seen it needed in other HF tasks. Do you have any idea why this might be happening?

The error is occurring because the code uses std::vector without including the header which defines it. It was not spotted before probably because of the recent upgrade of cpplint.

Please consider the following formatting changes to AliceO2Group#8210
@zhangbiao-phy

Copy link
Copy Markdown
CollaboratorAuthor

cpplint

@vkucera, I'm not sure why this error is occurring. I could add #include <vector> in the task, but I haven't seen it needed in other HF tasks. Do you have any idea why this might be happening?

The error is occurring because the code uses std::vector without including the header which defines it. It was not spotted before probably because of the recent upgrade of cpplint.

hi @vkucera, Thanks a lot! Done! but another error for checkov seems not related to the commited code

@vkucera

Copy link
Copy Markdown
Collaborator

cpplint

@vkucera, I'm not sure why this error is occurring. I could add #include <vector> in the task, but I haven't seen it needed in other HF tasks. Do you have any idea why this might be happening?

The error is occurring because the code uses std::vector without including the header which defines it. It was not spotted before probably because of the recent upgrade of cpplint.

hi @vkucera, Thanks a lot! Done! but another error for checkov seems not related to the commited code

Indeed, that one is unrelated.

Comment threadPWGHF/D2H/Tasks/taskLc.cxx Outdated
Co-authored-by: Vít Kučera <vit.kucera@cern.ch>
@vkucera

Copy link
Copy Markdown
Collaborator

Thanks @zhangbiao-phy for addressing my comments. Can you please test the latest version and confirm that it gives results identical with the results of the upstream version?

@zhangbiao-phy

Copy link
Copy Markdown
CollaboratorAuthor

Thanks @zhangbiao-phy for addressing my comments. Can you please test the latest version and confirm that it gives results identical with the results of the upstream version?

Hi @vkucera, Thanks for your review! I have tested the task with both MC and data using the old and new versions, and I have confirmed that both versions produce the same results.

@vkucera

Copy link
Copy Markdown
Collaborator

Thanks @zhangbiao-phy for addressing my comments. Can you please test the latest version and confirm that it gives results identical with the results of the upstream version?

Hi @vkucera, Thanks for your review! I have tested the task with both MC and data using the old and new versions, and I have confirmed that both versions produce the same results.

Thanks @zhangbiao-phy

@vkucera
vkucera merged commit d82639d into AliceO2Group:masterNov 3, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pwghfPWG-HF

Development

Successfully merging this pull request may close these issues.

4 participants

@zhangbiao-phy@alibuild@vkucera@mfaggin