Skip to content

PWGHF: Add Ds task and selector #941

Merged
vkucera merged 27 commits into
AliceO2Group:masterfrom
stefanopolitano:master
Sep 28, 2022
Merged

PWGHF: Add Ds task and selector #941
vkucera merged 27 commits into
AliceO2Group:masterfrom
stefanopolitano:master

Conversation

@stefanopolitano

Copy link
Copy Markdown
Collaborator

This PR contains the Ds -> phi pi -> K K pi task and selector.
This PR continues the discussion from AliceO2Group/O2Phyisics#96 and AliceO2Group/Run3Analysisvalidarion#323

The last comments from the previous PR were addressed and the macros were moved to the proper folder (D2H).

@stefanopolitano stefanopolitano changed the title PWGHF: Add Ds task and selector [PWGHF] Add Ds task and selector Jul 1, 2022
@vkucera vkucera changed the title [PWGHF] Add Ds task and selector PWGHF: Add Ds task and selector Jul 7, 2022
@stefanopolitano stefanopolitano force-pushed the master branch 4 times, most recently from ed9273d to aab07b3 Compare July 10, 2022 12:51
fgrosa
fgrosa previously approved these changes Jul 10, 2022

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

The change in the event filtering code is trivial, so I approve it already

Comment thread PWGHF/TableProducer/HFDsToKKPiCandidateSelector.cxx Outdated
Comment thread PWGHF/TableProducer/HFDsToKKPiCandidateSelector.cxx Outdated
Comment thread PWGHF/TableProducer/HFDsToKKPiCandidateSelector.cxx Outdated
Comment thread PWGHF/TableProducer/HFDsToKKPiCandidateSelector.cxx Outdated
Comment thread PWGHF/TableProducer/HFDsToKKPiCandidateSelector.cxx Outdated
Comment thread PWGHF/D2H/Tasks/taskDs.cxx Outdated
Comment thread PWGHF/DataModel/HFCandidateSelectionTables.h
Comment thread PWGHF/TableProducer/HFCandidateCreator3Prong.cxx Outdated
Comment thread PWGHF/TableProducer/HFCandidateCreator3Prong.cxx Outdated
Comment thread PWGHF/D2H/Tasks/taskDs.cxx Outdated
Comment thread PWGHF/DataModel/HFCandidateSelectionTables.h Outdated
@stefanopolitano stefanopolitano force-pushed the master branch 2 times, most recently from 0011483 to e788c70 Compare August 3, 2022 14:19
Comment thread PWGHF/TableProducer/HFTrackIndexSkimsCreator.cxx Outdated
vkucera
vkucera previously approved these changes Aug 16, 2022

@vkucera vkucera 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 for addressing the comments. The current version looks fine to me.

@vkucera vkucera marked this pull request as ready for review August 16, 2022 14:14
@alibuild

alibuild commented Aug 16, 2022

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/o2 for 8758008 at 2022-08-26 05:16:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/941-slc7_x86-64/0/EventFiltering/PWGHF/HFFilter.cxx:1133:76: error: 'DsToPiKK' is not a member of 'o2::aod::hf_cand_prong3::DecayType'
ninja: build stopped: subcommand failed.

Full log here.

@lvermunt

Copy link
Copy Markdown
Contributor

Hi @stefanopolitano

I had a look at your Ds code. For most I agree with @vkucera that it is ready for merging besides the error highlighted by alibuild. However I have also a doubt about how you use the RecoDecay::getMatchedMCRec() function for the Ds. And also on your modification for the Dplus one.

Now I might be wrong since I just had a quick look (@fgrosa, maybe you can comment as well), but it seems to me you tag all resonant Ds -> X -> KKpi, and not just Ds -> phi pi -> KKpi as we did for the Run 2 analyses. Is this intended? If so, I would say you need to tag the different resonant channels for possibly different efficiency corrections (as currently already done for Lc -> pKpi).

For the Dplus, your modification of the depthMax to 2 is correct to also take the resonant K*(892)0 channel into account (as we did in Run 2). However, since the pdg code of the K* is not specified, I guess it also tags the D+ -> K+ K0s -> K+ pi pi channel. And this latter channel you don't want I would say.

Finally, your call to RecoDecay::getMother() in taskDs.cxx should also get the depthMax = 2 right?

Cheers, Luuk

@lvermunt lvermunt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See #941 (comment) for my review.

@vkucera

vkucera commented Aug 22, 2022

Copy link
Copy Markdown
Collaborator

Hi @lvermunt

Finally, your call to RecoDecay::getMother() in taskDs.cxx should also get the depthMax = 2 right?

depthMax = -1 by default in getMother() so it does not make a difference for matched candidates.

@vkucera

vkucera commented Sep 1, 2022

Copy link
Copy Markdown
Collaborator

Hi @stefanopolitano , can you fix the error?

@lvermunt @xinyepeng Do you have any remaining comments?

@lvermunt

lvermunt commented Sep 1, 2022

Copy link
Copy Markdown
Contributor

Hi @vkucera, besides my two doubts on how the RecoDecay::getMatchedMCRec() function is used here which are not yet answered, and the error highlighted by alibuild, I don't have further comments.

Btw, if I understood correctly from a meeting earlier today, I think that Stefano is on holiday

@xinyepeng

Copy link
Copy Markdown
Contributor

Hi @vkucera, I agree with @lvermunt , nothing more from my side

@lvermunt

Copy link
Copy Markdown
Contributor

Hi @stefanopolitano, @fgrosa,

Thanks for your replies.

I don't really follow your reply on the KK invariant mass. This is of course a good selection to include in the analysis. However, I was speaking about the tagging of the MC candidates with the getMatchedMCRec() function for the efficiency correction. This should always check the correct decay channel (or tag the various resonant ones), and not depend on a KK invariant mass selection being enabled. I think Fabrizio says the same. Anyhow, fine for me to address this in a 2nd PR.

Regarding the D+, you are of course right @fgrosa. Sorry for introducing noise there.

This PR is for me thus good to be merged.

Cheers, Luuk

@fgrosa

fgrosa commented Sep 27, 2022

Copy link
Copy Markdown
Collaborator

Hi @lvermunt,

Hi @stefanopolitano, @fgrosa,

Thanks for your replies.

I don't really follow your reply on the KK invariant mass. This is of course a good selection to include in the analysis. However, I was speaking about the tagging of the MC candidates with the getMatchedMCRec() function for the efficiency correction. This should always check the correct decay channel (or tag the various resonant ones), and not depend on a KK invariant mass selection being enabled. I think Fabrizio says the same. Anyhow, fine for me to address this in a 2nd PR.

The massKK selection selects the Ds decaying into phi, without that selection we are not measuring Ds -> phi pi -> KK pi, but Ds -> X -> KKpi, where X is any intermediate state. Hence, as long as we don't have that selection the code is consistent without requiring the phi (but I agree that the immediately next step is to add it).

Regarding the D+, you are of course right @fgrosa. Sorry for introducing noise there.

This PR is for me thus good to be merged.

Cheers, Luuk

@lvermunt

Copy link
Copy Markdown
Contributor

Hi @lvermunt,

The massKK selection selects the Ds decaying into phi, without that selection we are not measuring Ds -> phi pi -> KK pi, but Ds -> X -> KKpi, where X is any intermediate state. Hence, as long as we don't have that selection the code is consistent without requiring the phi (but I agree that the immediately next step is to add it).

Sure, this I understand, and I agree the code is consistent enough (even though I'm also aware of your study for your PhD thesis that even with this massKK selection one still has a contamination of several percent by Ds -> f0(980)pi -> KKpi). My point was only that one shouldn't depend for the correct MC tagging on a selection that can be loosened or turned off. So that should be addressed in the future.

I approved and enabled the automatic merging. Unclear why the last pending check takes so long though

@vkucera

vkucera commented Sep 27, 2022

Copy link
Copy Markdown
Collaborator

@jgrosseo @ktf The errors in the macOS tests are unrelated. Can you please force the merge?

@jgrosseo

Copy link
Copy Markdown
Contributor

You should be able to merge, macOS is not required.

@jgrosseo

Copy link
Copy Markdown
Contributor

Maybe you miss the approval for EventFiltering ?

@vkucera vkucera merged commit 4db093d into AliceO2Group:master Sep 28, 2022
@vkucera

vkucera commented Sep 28, 2022

Copy link
Copy Markdown
Collaborator

Maybe you miss the approval for EventFiltering ?

Indeed

Sign up for free to 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.

8 participants