Skip to content

PWGHF: Adding D0 to taskFlowCharmHadrons, including Ml. - #5089

Merged
fgrosa merged 38 commits into
AliceO2Group:masterfrom
wuctlby:master
Mar 22, 2024
Merged

PWGHF: Adding D0 to taskFlowCharmHadrons, including Ml.#5089
fgrosa merged 38 commits into
AliceO2Group:masterfrom
wuctlby:master

Conversation

@wuctlby

Copy link
Copy Markdown
Contributor

No description provided.

@fgrosafgrosa 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.

Hi @Libeiyu thanks a lot for the development! I have a few comments that have to be addressed before merging, see below

Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
@wuctlby
wuctlby requested review from fgrosa and vkuceraMarch 19, 2024 08:58

@fgrosafgrosa 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.

Hi @Libeiyu thanks a lot for implementing my comments! I just have two minor suggestions remaining, then for me it can be merged.
Also notifying @stefanopolitano that the enum for the centrality estimator has been modified.

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

Copy link
Copy Markdown
Collaborator
  • runFlowAnalysis is called with a collision iterator and a partition as arguments but I don't see any slicing of the partition by the collision index.
  • Also the second template argument of runFlowAnalysis should be deduced from the partition type but you are providing it explicitly.

Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
wuctlbyand others added 4 commits March 20, 2024 14:17
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
@wuctlby
wuctlby requested review from fgrosa and vkuceraMarch 21, 2024 04:49
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
Comment threadPWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx Outdated
@vkucera

vkucera commented Mar 21, 2024

Copy link
Copy Markdown
Collaborator
  • runFlowAnalysis is called with a collision iterator and a partition as arguments but I don't see any slicing of the partition by the collision index.

  • Also the second template argument of runFlowAnalysis should be deduced from the partition type but you are providing it explicitly.

You reacted with 👍🏻 but I don't see any related changes.

@vkucera

Copy link
Copy Markdown
Collaborator

@Libeiyu Sorry for a blunt question, but who are you? I cannot find anyone called Libeiyu in ALICE.

@wuctlby

Copy link
Copy Markdown
ContributorAuthor
  • runFlowAnalysis is called with a collision iterator and a partition as arguments but I don't see any slicing of the partition by the collision index.
  • Also the second template argument of runFlowAnalysis should be deduced from the partition type but you are providing it explicitly.

You reacted with 👍🏻 but I don't see any related changes.

  • For the first point, they should be already sliced since the collision and the d meson tables possess the global index.

  • For the second point, I thought you were talking about the DecayChannel. I already added a new commit about this.

@wuctlby

Copy link
Copy Markdown
ContributorAuthor

@Libeiyu Sorry for a blunt question, but who are you? I cannot find anyone called Libeiyu in ALICE.

Yes. "Libeiyu" is just a username, or online alias. My true name is Chuntai

@vkucera

Copy link
Copy Markdown
Collaborator
  • runFlowAnalysis is called with a collision iterator and a partition as arguments but I don't see any slicing of the partition by the collision index.
  • Also the second template argument of runFlowAnalysis should be deduced from the partition type but you are providing it explicitly.

You reacted with 👍🏻 but I don't see any related changes.

* For the first point, they should be already sliced since the collision and the d meson tables possess the global index.
* For the second point, I thought you were talking about the `DecayChannel`. I already added a new commit about this.
  • Presence of global indices does not imply any slicing. You have to do it explicitly.
  • I was talking about the second template argument, which is T1.

@vkucera

vkucera commented Mar 21, 2024

Copy link
Copy Markdown
Collaborator

@Libeiyu Sorry for a blunt question, but who are you? I cannot find anyone called Libeiyu in ALICE.

Yes. "Libeiyu" is just a username, or online alias. My true name is Chuntai

OK, so please update your GitHub profile to show your real name and add an \author line in the code documentation.

@stefanopolitano

Copy link
Copy Markdown
Collaborator
  • runFlowAnalysis is called with a collision iterator and a partition as arguments but I don't see any slicing of the partition by the collision index.
  • Also the second template argument of runFlowAnalysis should be deduced from the partition type but you are providing it explicitly.

You reacted with 👍🏻 but I don't see any related changes.

* For the first point, they should be already sliced since the collision and the d meson tables possess the global index.
* For the second point, I thought you were talking about the `DecayChannel`. I already added a new commit about this.
* Presence of global indices does not imply any slicing. You have to do it explicitly.
* I was talking about the _second_ template argument, which is `T1`.

Hi @vkucera ! Sorry for jumping in and thanks a lot for pointing this out. Indeed, as @Libeiyu said, I thought the iterator was sufficient to point to the "sliced" D mesons too. We will fix this by adding a preslice on the 2- and 3-prongs tables and a sliceby() in the process functions. However, I think this can be done in a different PR since it is a bit out of topic with respect to this one. What do you think?

@vkucera

Copy link
Copy Markdown
Collaborator
  • runFlowAnalysis is called with a collision iterator and a partition as arguments but I don't see any slicing of the partition by the collision index.
  • Also the second template argument of runFlowAnalysis should be deduced from the partition type but you are providing it explicitly.

You reacted with 👍🏻 but I don't see any related changes.

* For the first point, they should be already sliced since the collision and the d meson tables possess the global index.
* For the second point, I thought you were talking about the `DecayChannel`. I already added a new commit about this.
* Presence of global indices does not imply any slicing. You have to do it explicitly.
* I was talking about the _second_ template argument, which is `T1`.

Hi @vkucera ! Sorry for jumping in and thanks a lot for pointing this out. Indeed, as @Libeiyu said, I thought the iterator was sufficient to point to the "sliced" D mesons too. We will fix this by adding a preslice on the 2- and 3-prongs tables and a sliceby() in the process functions. However, I think this can be done in a different PR since it is a bit out of topic with respect to this one. What do you think?

Hi @stefanopolitano , fine with me, if you link this PR in the description of the new one that fixes the raised points.

@fgrosa
fgrosa merged commit d454803 into AliceO2Group:masterMar 22, 2024
christianreckziegel pushed a commit to christianreckziegel/O2Physics that referenced this pull request May 2, 2024
…#5089)
* version 2: taskFlowCharmHadrons
* No selection flag in Thn
* No selection Flag
* Add D0 to taskFlowCharmHadrons, , including Ml
* Please consider the following formatting changes
* #
* #
* correct float type
* correct float type
* prongNum >> nProngs
* Please consider the following formatting changes
* using rabs instead of spaces
* Update PWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx
Call getQvecDtracks without if
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
* Update PWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx
New name for decaychannel
Co-authored-by: Vít Kučera <vit.kucera@cern.ch>
* Update taskFlowCharmHadrons.cxx
New name for p*Track
* Update taskFlowCharmHadrons.cxx
* Update taskFlowCharmHadrons.cxx
* Update taskFlowCharmHadrons.cxx
* DecayChannel channel >> int channel
* Add enum CentralityEstimator and fix to QVecEstimator
* Update taskFlowCharmHadrons.cxx
Fix the param in fillThn
* Update taskFlowCharmHadrons.cxx
* The two mass hypotheses handled similarly for all particle species
* Please consider the following formatting changes
* wML >> WML
* no nProngs
* Update PWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
* Update PWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
* No Lc+
* int >> DecayChannel
* Channel >> channel
* Add author
---------
Co-authored-by: ALICE Action Bot <alibuild@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Vít Kučera <vit.kucera@cern.ch>
hahassan7 pushed a commit to hahassan7/O2Physics that referenced this pull request May 17, 2024
…#5089)
* version 2: taskFlowCharmHadrons
* No selection flag in Thn
* No selection Flag
* Add D0 to taskFlowCharmHadrons, , including Ml
* Please consider the following formatting changes
* #
* #
* correct float type
* correct float type
* prongNum >> nProngs
* Please consider the following formatting changes
* using rabs instead of spaces
* Update PWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx
Call getQvecDtracks without if
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
* Update PWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx
New name for decaychannel
Co-authored-by: Vít Kučera <vit.kucera@cern.ch>
* Update taskFlowCharmHadrons.cxx
New name for p*Track
* Update taskFlowCharmHadrons.cxx
* Update taskFlowCharmHadrons.cxx
* Update taskFlowCharmHadrons.cxx
* DecayChannel channel >> int channel
* Add enum CentralityEstimator and fix to QVecEstimator
* Update taskFlowCharmHadrons.cxx
Fix the param in fillThn
* Update taskFlowCharmHadrons.cxx
* The two mass hypotheses handled similarly for all particle species
* Please consider the following formatting changes
* wML >> WML
* no nProngs
* Update PWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
* Update PWGHF/D2H/Tasks/taskFlowCharmHadrons.cxx
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
* No Lc+
* int >> DecayChannel
* Channel >> channel
* Add author
---------
Co-authored-by: ALICE Action Bot <alibuild@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Vít Kučera <vit.kucera@cern.ch>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@wuctlby@vkucera@stefanopolitano@fgrosa@alibuild