PWGHF: Fix for Signal status in MC Rec in D0-hadron correlations and implement event mixing by samrangy · Pull Request #3103 · AliceO2Group/O2Physics

samrangy · 2023-07-18T14:43:18Z

No description provided.

Please consider the following formatting changes to AliceO2Group#3103

mmazzilli

Hello @samrangy, this PR is fine with me. So for the HFC it can be approved.
Please be aware that there might be conflicts with the PR #3115 (not yet merged for now), but @vkucera can comment on this.
Cheers,
Marianna

vkucera

Hi @samrangy , please

fix the structure of the code (order of members),
fix the naming (tasks and types),
fix the types of the configurables to avoid unnecessary casting in filters
fix hard-coded PDG codes
fix hard-coded values of signalStatus.

samrangy · 2023-07-20T16:45:30Z

Hi @vkucera, thanks for your comments. Could you please elaborate the first two points? It would be helpful if you provide example.

vkucera · 2023-07-21T11:55:14Z

On the structure: You declare new types and filters in between definitions of member methods. That makes the code hard to read and violates the HF coding conventions on the order of struct members.
On the naming: Your names of types (e.g. myCollisionsMcGen) do not follow the O2 capitalisation conventions and are too vague. The task HfD0Selection does not follow the HF naming conventions for auxiliary tasks.

Please consider the following formatting changes to AliceO2Group#3103

vkucera · 2023-07-26T16:50:30Z

 {"hDeltaPhiPtIntSignalRegion", stringDHadron + stringSignal + stringDeltaPhi + "entries", {HistType::kTH1F, {{nPhiBinsCorrelations, phiMinCorrelations, phiMaxCorrelations}}}},
 {"hCorrel2DPtIntSignalRegion", stringDHadron + stringSignal + stringDeltaPhi + stringDeltaEta + "entries", {HistType::kTH2F, {{nPhiBinsCorrelations, phiMinCorrelations, phiMaxCorrelations}, {200, -10., 10.}}}},
- {"hCorrel2DVsPtSignalRegion", stringDHadron + stringSignal + stringDeltaPhi + stringDeltaEta + stringPtD + stringPtHadron + "entries", {HistType::kTHnSparseD, {{nPhiBinsCorrelations, phiMinCorrelations, phiMaxCorrelations}, {nEtaBinsCorrelations, etaMinCorrelations, etaMaxCorrelations}, {10, 0., 10.}, {11, 0., 11.}}}}, // note: axes 3 and 4 (the pT) are updated in the init()
+ {"hCorrel2DVsPtSignalRegion", stringDHadron + stringSignal + stringDeltaPhi + stringDeltaEta + stringPtD + stringPtHadron + "entries", {HistType::kTHnSparseD, {{nPhiBinsCorrelations, phiMinCorrelations, phiMaxCorrelations}, {nEtaBinsCorrelations, etaMinCorrelations, etaMaxCorrelations}, {10, 0., 10.}, {11, 0., 11.}, {9, 0., 9.}}}}, // note: axes 3 and 4 (the pT) are updated in the init()


Avoid repeating the same hard-coded axes.

You still use e.g. {nPhiBinsCorrelations, phiMinCorrelations, phiMaxCorrelations}, {200, -10., 10.} at many places. Define a single axis variable for each frequently used axis.

Please consider the following formatting changes to AliceO2Group#3103

vkucera · 2023-08-07T14:28:15Z


 /// D0-h correlation pair builder - for real data and data-like analysis (i.e. reco-level w/o matching request via MC truth)
- void processData(aod::Collision const& collision, soa::Join<aod::Tracks, aod::TracksDCA>& tracks, soa::Join<aod::HfCand2Prong, aod::HfSelD0> const& candidates)
+ void processData(soa::Join<aod::Collisions, aod::Mults>::iterator const& collision, soa::Join<aod::Tracks, aod::TracksDCA>& tracks, soa::Join<aod::HfCand2Prong, aod::HfSelD0> const& candidates)


Still not fixed in many process functions.

vkucera · 2023-08-07T14:34:51Z

 {"hDeltaPhiPtIntSignalRegion", stringDHadron + stringSignal + stringDeltaPhi + "entries", {HistType::kTH1F, {{nPhiBinsCorrelations, phiMinCorrelations, phiMaxCorrelations}}}},
 {"hCorrel2DPtIntSignalRegion", stringDHadron + stringSignal + stringDeltaPhi + stringDeltaEta + "entries", {HistType::kTH2F, {{nPhiBinsCorrelations, phiMinCorrelations, phiMaxCorrelations}, {200, -10., 10.}}}},
- {"hCorrel2DVsPtSignalRegion", stringDHadron + stringSignal + stringDeltaPhi + stringDeltaEta + stringPtD + stringPtHadron + "entries", {HistType::kTHnSparseD, {{nPhiBinsCorrelations, phiMinCorrelations, phiMaxCorrelations}, {nEtaBinsCorrelations, etaMinCorrelations, etaMaxCorrelations}, {10, 0., 10.}, {11, 0., 11.}}}}, // note: axes 3 and 4 (the pT) are updated in the init()
+ {"hCorrel2DVsPtSignalRegion", stringDHadron + stringSignal + stringDeltaPhi + stringDeltaEta + stringPtD + stringPtHadron + "entries", {HistType::kTHnSparseD, {{nPhiBinsCorrelations, phiMinCorrelations, phiMaxCorrelations}, {nEtaBinsCorrelations, etaMinCorrelations, etaMaxCorrelations}, {10, 0., 10.}, {11, 0., 11.}, {9, 0., 9.}}}}, // note: axes 3 and 4 (the pT) are updated in the init()


You still use e.g. {nPhiBinsCorrelations, phiMinCorrelations, phiMaxCorrelations}, {200, -10., 10.} at many places. Define a single axis variable for each frequently used axis.

Please consider the following formatting changes to AliceO2Group#3103

vkucera · 2023-08-08T08:33:05Z

Please @samrangy do not force-push when you add commits! It is explicitly discouraged in the guidelines because it makes the review more complicated.

alibuild · 2023-08-08T09:32:10Z

Error while checking build/O2Physics/o2 for 5b7866a at 2023-08-08 11:32:

## sw/BUILD/O2Physics-latest/log
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
[ERROR] Function RecoDecay::getMassPDG is deprecated and will be removed soon.
[ERROR] Please use the Mass function in the O2DatabasePDG service instead.
[ERROR] See the example of usage in Tutorials/src/usingPDGService.cxx.
/sw/SOURCES/O2Physics/3103-slc7_x86-64/0/PWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx:112:115: error: 'axisDetlaEta' was not declared in this scope; did you mean 'axisDeltaEta'?
/sw/SOURCES/O2Physics/3103-slc7_x86-64/0/PWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx:113:115: error: 'axisDetlaPhi' was not declared in this scope; did you mean 'axisDeltaPhi'?
/sw/SOURCES/O2Physics/3103-slc7_x86-64/0/PWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx:114:133: error: 'axisDetlaPhi' was not declared in this scope; did you mean 'axisDeltaPhi'?
/sw/SOURCES/O2Physics/3103-slc7_x86-64/0/PWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx:114:149: error: 'axisDetlaEta' was not declared in this scope; did you mean 'axisDeltaEta'?
/sw/SOURCES/O2Physics/3103-slc7_x86-64/0/PWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx:115:167: error: 'axisDetlaPhi' was not declared in this scope; did you mean 'axisDeltaPhi'?
/sw/SOURCES/O2Physics/3103-slc7_x86-64/0/PWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx:115:183: error: 'axisDetlaEta' was not declared in this scope; did you mean 'axisDeltaEta'?
[0 more errors; see full log]

Full log here.

samrangy · 2023-08-08T13:43:15Z

Every time I try to push it, I receive the following error, So I had to force it.

[rejected] master -> master (fetch first)
error: failed to push some refs to 'https://github.com/samrangy/O2Physics'

Please consider the following formatting changes to AliceO2Group#3103

vkucera · 2023-08-08T13:53:26Z

Every time I try to push it, I receive the following error, So I had to force it.
[rejected] master -> master (fetch first) error: failed to push some refs to 'https://github.com/samrangy/O2Physics'

That is because you modify your branch on GitHub with the formatting PR and then you try to push to the modified branch your local commits without updating your local branch first. You have to sync your local branch with the remote branch before pushing new commits to it.

vkucera · 2023-08-09T13:39:44Z

Thanks @samrangy

…implement event mixing (AliceO2Group#3103)

samrangy requested review from alibuild, deepathoms, fcolamar, ginnocen, mmazzilli and vkucera as code owners July 18, 2023 14:43

samrangy added a commit to samrangy/O2Physics that referenced this pull request Jul 18, 2023
Merge pull request#16from alibuild/alibot-cleanup-3103
54c3a74
Please consider the following formatting changes to AliceO2Group#3103

mmazzilli previously approved these changes Jul 20, 2023
View reviewed changes

vkucera requested changes Jul 20, 2023
View reviewed changes

samrangy dismissed mmazzilli’s stale review via dd9d198July 21, 2023 16:56

samrangy force-pushed the master branch from 54c3a74 to dd9d198Compare July 21, 2023 16:56

samrangy added a commit to samrangy/O2Physics that referenced this pull request Jul 21, 2023
Merge pull request#18from alibuild/alibot-cleanup-3103
0c89edf
Please consider the following formatting changes to AliceO2Group#3103

vkucera requested changes Jul 26, 2023
View reviewed changes

samrangy force-pushed the master branch from 0c89edf to 88960e1Compare July 31, 2023 15:38

samrangy requested a review from adubla as a code owner July 31, 2023 15:38

samrangy added a commit to samrangy/O2Physics that referenced this pull request Jul 31, 2023
Merge pull request#19from alibuild/alibot-cleanup-3103
99f68a5
Please consider the following formatting changes to AliceO2Group#3103

samrangy force-pushed the master branch from 99f68a5 to 7a25b35Compare August 4, 2023 08:10

samrangy added a commit to samrangy/O2Physics that referenced this pull request Aug 4, 2023
Merge pull request#20from alibuild/alibot-cleanup-3103
dfeeb18
Please consider the following formatting changes to AliceO2Group#3103

vkucera reviewed Aug 7, 2023
View reviewed changes

samrangy force-pushed the master branch from dfeeb18 to 7ac06feCompare August 7, 2023 16:38

samrangy added a commit to samrangy/O2Physics that referenced this pull request Aug 7, 2023
Merge pull request#21from alibuild/alibot-cleanup-3103
3b4f028
Please consider the following formatting changes to AliceO2Group#3103

vkucera requested changes Aug 8, 2023
View reviewed changes

Comment threadPWGHF/HFC/TableProducer/correlatorD0Hadrons.cxx

vkucera requested changes Aug 8, 2023
View reviewed changes

Comment threadPWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx Outdated

vkucera requested changes Aug 8, 2023
View reviewed changes

Comment threadPWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx
Comment threadPWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx Outdated
Comment threadPWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx Outdated
Comment threadPWGHF/HFC/Tasks/taskCorrelationD0Hadrons.cxx Outdated

samrangy added 2 commits August 8, 2023 18:27

resolve conflict
7a0588e

fix for else braces
a1431f3

samrangy added 7 commits August 8, 2023 18:27

correction for enum
1d03bd8

naming modification
7e787e5

Addressing Vit's comments
95cc8a2

addressing Vit's comments
7f6f4c2

changing names for signal region and fix sideband array
da434b2

Fix aliases and axis variables
a18c344

remove extra functions and fix comments
67d7654

samrangy force-pushed the master branch from 5b7866a to 67d7654Compare August 8, 2023 13:41

Please consider the following formatting changes
bccc2d3

Merge pull request#22from alibuild/alibot-cleanup-3103
a63a457
Please consider the following formatting changes to AliceO2Group#3103

vkucera approved these changes Aug 9, 2023
View reviewed changes

vkucera merged commit be18e67 into AliceO2Group:masterAug 9, 2023

hahassan7 pushed a commit to hahassan7/O2Physics that referenced this pull request Oct 6, 2023
PWGHF: Fix for Signal status in MC Rec in D0-hadron correlations and …
9750a7d
…implement event mixing (AliceO2Group#3103)

hahassan7 pushed a commit to hahassan7/O2Physics that referenced this pull request Oct 7, 2023
PWGHF: Fix for Signal status in MC Rec in D0-hadron correlations and …
85a6e28
…implement event mixing (AliceO2Group#3103)

zconesa pushed a commit to zconesa/O2Physics that referenced this pull request Oct 27, 2023
PWGHF: Fix for Signal status in MC Rec in D0-hadron correlations and …
dab1eeb
…implement event mixing (AliceO2Group#3103)

chengtt0406 pushed a commit to chengtt0406/O2Physics that referenced this pull request Dec 6, 2023
PWGHF: Fix for Signal status in MC Rec in D0-hadron correlations and …
9ae587f
…implement event mixing (AliceO2Group#3103)

Uh oh!

Conversation

samrangy commented Jul 18, 2023

Uh oh!

mmazzilli left a comment

Choose a reason for hiding this comment

Uh oh!

vkucera left a comment

Choose a reason for hiding this comment

Uh oh!

samrangy commented Jul 20, 2023

Uh oh!

vkucera commented Jul 21, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vkuceraJul 26, 2023

Choose a reason for hiding this comment

Uh oh!

vkuceraAug 7, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vkuceraAug 7, 2023

Choose a reason for hiding this comment

Uh oh!

vkuceraAug 7, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vkucera commented Aug 8, 2023

Uh oh!

alibuild commented Aug 8, 2023

Uh oh!

samrangy commented Aug 8, 2023

Uh oh!

vkucera commented Aug 8, 2023

Uh oh!

vkucera commented Aug 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants