PWGHF: Adding first implementation of D0-D0bar correlation code (data/MC-reco/MC-kine) by fcolamar · Pull Request #5450 · AliceO2Group/AliceO2

fcolamar · Feb 11, 2021

Implementation of analysis task for D0-D0bar correlations.
Allows running in data-like, MC-reco and MC-kine modes.
ULS and LS analysis enables.

fcolamar · Feb 11, 2021

Hi @vkucera, @ginnocen, I'm opening this draft pull request for the D0-D0bar tasks integration, let me know for any comment

vkucera

Hi @fcolamar .
Please fix the naming so that it complies with the conventions.

fcolamar · Feb 18, 2021

Hi @vkucera, I modified the task names, which were inadequate, and the filename according to the coding conventions.
Let me know if there's some other renaming that I missed, thanks

vkucera · Feb 18, 2021

Thanks, please have a look at the General Naming Rules. That might be clearer.

ginnocen · Feb 18, 2021

@vkucera but what is the specific problem in this case?

vkucera · Feb 18, 2021

Thanks, please have a look at the General Naming Rules. That might be clearer.

There are still some variables, histogram names and other pieces, like underscores, to fix. Also missing braces.
It might be better to fix those before starting with the code comments.

vkucera · Feb 19, 2021

Thanks for the update. It still does not comply with the conventions but I will comment together with other suggestions.

vkucera

Most comments on the first task apply to the others too.
The tasks share many hard-coded constants which should be defined only once.

vkucera · Feb 19, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+  double dPhi = phiDbar - phiD;
+
+  if (dPhi < -o2::constants::math::PI / 2.)
+    dPhi = dPhi + 2. * o2::constants::math::PI;
+  if (dPhi > 3. * o2::constants::math::PI / 2.)
+    dPhi = dPhi - 2. * o2::constants::math::PI;
+
+  return dPhi;


You can replace this with

return RecoDecay::constrainAngle(phiDbar - phiD, -o2::constants::math::PI / 2.);

See #5521

Done, though I'm waiting for the merging of the #5521 pull request

vkucera · Feb 19, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+  double phi = std::atan2((yVertex2 - yVertex1), (xVertex2 - xVertex1));
+
+  if (phi < 0.)
+    phi = phi + 2. * o2::constants::math::PI;
+  if (phi > 2. * o2::constants::math::PI)
+    phi = phi - 2. * o2::constants::math::PI;
+
+  return phi;


You can replace this with

return RecoDecay::Phi(xVertex2 - xVertex1, yVertex2 - yVertex1);

vkucera · Feb 19, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+/// D0 analysis task - for real data and data-like analysis (i.e. reco-level w/o matching request via MC truth)
+struct TaskD0D0barCorrelation {
+
+  double maxEtaCut = 5., PtThresholdForMaxEtaCut = 10.; //for hDDbarVsEtaCut, gives eta increment of 0.1 and pt thr increments of 0.5


Is maxEtaCut supposed to be different from cutEtaCandMax?

Should maxEtaCut, PtThresholdForMaxEtaCut be configurable or constant?

The increments are independent of these values. So the comment is incorrect.

The actual increments of ptCut and etaCut are hard-coded, whereas the bin widths of hDDbarVsEtaCut are defined with separate binning factors. They should be linked via constants.

Yes because cutEtaCandMax is a selection done on the candidates (which at some points could be changed to a y selection), while maxEtaCut defines the limit of the scan for the potential detector acceptance

so I was initially in doubt, but I think that with values large enough, as the current, they can be kept as constant (modified it, but moved outside the tasks to avoid multiple definition)

yes right, it's a leftover from a previous version, with dynamic increment depending on those variables, I forgot changing the comment

ok, modified

vkucera · Feb 19, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+      if (cutEtaCandMax >= 0. && std::abs(candidate1.eta()) > cutEtaCandMax)
+        continue;
+      if (cutPtCandMin >= 0. && std::abs(candidate1.pt()) < cutPtCandMin)
+        continue;
+      //check decay channel flag for candidate1


Add readability-braces-around-statements.

Done (also for the other equivalent cases)

vkucera · Feb 19, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+    {{"hmass", "D0,D0bar candidates;inv. mass (#pi K) (GeV/#it{c}^{2});entries", {HistType::kTH1F, {{200, 1., 3.}}}},
+     {"hmassD0", "D0,D0bar candidates;inv. mass D0 only (#pi K) (GeV/#it{c}^{2});entries", {HistType::kTH1F, {{200, 1., 3.}}}},
+     {"hmassD0bar", "D0,D0bar candidates;inv. mass D0bar only (#pi K) (GeV/#it{c}^{2});entries", {HistType::kTH1F, {{200, 1., 3.}}}},
+     {"hmass2DCorrelationPairs", "D0,D0bar candidates 2D;inv. mass D0 only (#pi K) (GeV/#it{c}^{2});inv. mass D0bar only (#pi K) (GeV/#it{c}^{2});entries", {HistType::kTH2F, {{200, 1., 3.}, {200, 1., 3.}}}},
+     {"hptcand", "D0,D0bar candidates;candidate #it{p}_{T} (GeV/#it{c});entries", {HistType::kTH1F, {{100, 0., 10.}}}},
+     {"hptprong0", "D0,D0bar candidates;prong 0 #it{p}_{T} (GeV/#it{c});entries", {HistType::kTH1F, {{100, 0., 10.}}}},
+     {"hptprong1", "D0,D0bar candidates;prong 1 #it{p}_{T} (GeV/#it{c});entries", {HistType::kTH1F, {{100, 0., 10.}}}},
+     {"hselectionstatus", "D0,D0bar candidates;selection status;entries", {HistType::kTH1F, {{5, -0.5, 4.5}}}},


Better to stick to the camelCase convention.
(We have to fix it in the existing tasks so it's better if the new code is already ok.)

Ah ok, I had noticed that these names was not following the convention, but since was present in other tasks I hadn't modified them in mine. Done now

vkucera · Feb 19, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+
+  Filter filterSelectCandidates = (aod::hf_selcandidate_d0::isSelD0 >= d_selectionFlagD0 || aod::hf_selcandidate_d0::isSelD0bar >= d_selectionFlagD0bar);
+
+  void process(aod::Collision const& collision, soa::Filtered<aod::Big2Prong> const& candidates, aod::BigTracks const& tracks)


The track table does not seem to be used anywhere.

You are right: this was needed in the previous version, before the grouping of candidates with collisions, to ensure that D0 mesons were correlated with D0bar mesons from the same event (and the collision was tracked via the information stored in the daughter track table), but after implementing the grouping that check has been removed, and the track table is not used anymore. I'm removing it.

vkucera · Feb 19, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+
+    //loop over particles at MC gen level
+    for (auto& particle1 : particlesMC) {
+      if (std::abs(particle1.pdgCode()) == 4) { //c or cbar quark found


Why don't you just continue if it isn't 4?

I modified as you proposed since it's cleaner

vkucera · Feb 19, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+        registry.fill(HIST("hptcandMCGen"), particle1.pt());
+        registry.fill(HIST("hEtaMCGen"), particle1.eta());
+        registry.fill(HIST("hPhiMCGen"), particle1.phi());


Do I understand correctly that (only) these histograms should be filled for each c(bar) and each fragmentation stage?

Ah thanks, this is a mistake indeed: it's due to a leftover from a previous version, where I had two version of these plots (with and without inclusion of fragmentation quarks) to check the effects on those distributions from double-countings due to fragmentation.
I deleted one version after doing the check, removed the suffix from the histogram names, but left the 'wrong' version (before fragmentation check).
Also, at this point it's useless to have the instruction that skips the fragmentation (L691-692 of old commit) so late in the task, with a couple of if instructions before, so I moved that instruction at the beginning, and removed the if's.

vkucera · Feb 19, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+        //get corresponding gen-level D0, if exists, and evaluate gen-rec phi-difference with two approaches
+        if (std::abs(candidate1.flagMCMatchRec()) == 1 << D0ToPiK) {                                                     //ok to keep both D0 and D0bar
+          int indexGen = RecoDecay::getMother(particlesMC, candidate1.index0_as<aod::BigTracksMC>().label(), 421, true); //MC-gen corresponding index for MC-reco candidate
+          if (indexGen > 0) {


Why if (indexGen > 0)?

What I want to do is to enaure that what follows is done only if a gen-reco match is ensured, but looking how the returned variable is initialised in RecoDecay::getMother, I think I should set the condition to > -1, rather than 0 (this is what I'm doing now, please correct me if not ok)

The gen-rec match is ensured already by requiring:

if (std::abs(candidate1.flagMCMatchRec()) == 1 << D0ToPiK) {

Therefore getMother will always return a valid index.

Ok, removed the condition

vkucera · Feb 19, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+            registry.fill(HIST("hDifferenceGenPhiStdPhi"), getDeltaPhi(phi1Std, phi1Gen), pt1);
+            registry.fill(HIST("hDifferenceGenPhiByVtxPhi"), getDeltaPhi(phi1ByVtx, phi1Gen), pt1);


Shouldn't resolution be constrained to range [-π, +π]?

I preferred having a full version with positives and negatives which acted also as control plot in case of spotting potential asymmetries (though remote), then one can always reflect while processing the plot.

The point here is that getDeltaPhi returns angles in range [-π/2, 3*π/2], which makes no sense for resolution.
Resolution expresses how far the rec. value is from the gen. value and that can never be larger than π for an angle.

Ah I misread the comment (read [0,pi] instead). Ok then, I modified the code accordingly

vkucera · Mar 1, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

-          if (partMothPDG == 4)
-            continue;
-          registry.fill(HIST("hcountD0triggersMCGen"), 0); //to count trigger c quark (for normalisation)
+      if (!std::abs(particle1.pdgCode()) == 4) { //search c or cbar particles


This is not gonna work. Should be:

if (std::abs(particle1.pdgCode()) != 4) {

vkucera · Mar 1, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+        //get corresponding gen-level D0, if exists, and evaluate gen-rec phi-difference with two approaches
+        if (std::abs(candidate1.flagMCMatchRec()) == 1 << D0ToPiK) {                                                     //ok to keep both D0 and D0bar
+          int indexGen = RecoDecay::getMother(particlesMC, candidate1.index0_as<aod::BigTracksMC>().label(), 421, true); //MC-gen corresponding index for MC-reco candidate
+          if (indexGen > 0) {


The gen-rec match is ensured already by requiring:

if (std::abs(candidate1.flagMCMatchRec()) == 1 << D0ToPiK) {

Therefore getMother will always return a valid index.

vkucera · Mar 1, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+            registry.fill(HIST("hDeltaEtaVsPtMCGen"), pt1, pt2, deltaEta);
+            registry.fill(HIST("hDeltaPhiVsPtMCGen"), pt1, pt2, deltaPhi);
+            registry.fill(HIST("hDeltaPtDDbarMCGen"), pt2 - pt1);
+            registry.fill(HIST("hDeltaPtMaxMinMCGen"), std::max(particle2.pt(), particle1.pt()) - std::min(particle2.pt(), particle1.pt()));


Changed as done for previous occurrences

Committed updated version which satisfies the new requests for adaptAnalysisTask (#PR5558).
No modifications are needed for the charge()->sign() update (#PR5611)

fcolamar · Mar 8, 2021

Hello all, is there any further comment on this PR?
Thanks!

vkucera · Mar 8, 2021

Hi @fcolamar , your branch is 247 commits behind dev. Could you please update it and check that your code works fine with the latest O2?

merging with changes from remote dev

fcolamar · Mar 8, 2021

Hi Vit, I updated the branch with the new dev commits, the only needed modification was an update to the CMakeLists.txt which was indeed modified by Fabio Catalano this morning. Tried to compile and it works fine (I couldn't test the code yet due to errors when running the validation framework, though I don't expect problems)

vkucera · Mar 8, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+      if (cutPtCandMin >= 0. && std::abs(candidate1.pt()) < cutPtCandMin) {
+        continue;


Absolute value of pT?

Right, copy-paste from above selection on eta done without thinking... Removed the abs (here and later on)

vkucera · Mar 8, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+
+/// definition of variables for D0D0bar pairs vs eta acceptance studies (hDDbarVsEtaCut, in data-like, MC-reco and MC-kine tasks)
+const double maxEtaCut = 5.;
+const double PtThresholdForMaxEtaCut = 10.;


ptThresholdForMaxEtaCut

vkucera · Mar 8, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+const double PtThresholdForMaxEtaCut = 10.;
+const double incrementEtaCut = 0.1;
+const double incrementPtThreshold = 0.5;
+const double epsilon = 10E-5;


Is it supposed to be 1e-4?

Modified to 1E-5

vkucera · Mar 8, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+///
+/// \author Fabio Colamaria <fabio.colamaria@ba.infn.it>, INFN Bari
+
+#include <cmath>


Why is this needed?

Removed (leftover from previous version with math operations that were directly implemented here)

vkucera · Mar 8, 2021

Analysis/Tasks/PWGHF/taskD0D0barCorrelation.cxx

+// the following is needed to extend the standard candidate table, and allow grouping candidate table by collisions
+namespace o2::aod
+{
+namespace hf_2prong_correlation
+{
+DECLARE_SOA_INDEX_COLUMN(Collision, collision);
+} // namespace hf_2prong_correlation
+DECLARE_SOA_TABLE(HF2ProngCollis, "AOD", "COLLID_2PR", aod::hf_2prong_correlation::CollisionId);
+
+using Big2Prong = soa::Join<aod::HfCandProng2, aod::HFSelD0Candidate, aod::HF2ProngCollis>;
+using Big2ProngMC = soa::Join<aod::HfCandProng2, aod::HFSelD0Candidate, aod::HfCandProng2MCRec, aod::HF2ProngCollis>;
+} // namespace o2::aod
+
+// preliminary task to fill the column index to the extended candidate table
+struct CreateBig2Prong {
+
+  Produces<aod::HF2ProngCollis> create2ProngIndexCollColumn;
+  void process(aod::HfCandProng2 const& candidates, aod::Tracks const& tracks)
+  {
+    for (auto& candidate : candidates) {
+      int indexColl = candidate.index0_as<aod::Tracks>().collisionId(); //takes index of collision from first D daughter
+      create2ProngIndexCollColumn(indexColl);
+    }
+  }
+};


This should be removed once #5469 gets merged.

Sure, for now I left it in place

Since #5469 was merged this morning, I adapted the task and committed the updated version

fcolamar · Mar 11, 2021

Draft PR ready for new review

…ded centrally)

fcolamar · Mar 18, 2021

Adapted to merging of #5469 PR, ready for review

…lysis tasks

fcolamar · Apr 15, 2021

@ginnocen @vkucera I've just committed the new version of the D0D0bar code, with the full restructuring as discussed during yesterday's meeting

github-actions · May 16, 2021

This PR did not have any update in the last 30 days. Is it still needed? Unless further action in will be closed in 5 days.

vkucera · May 18, 2021

@fcolamar Shall we then close this PR in favour of #6124?

fcolamar · May 19, 2021

Hi @vkucera, in my opinion is would be better, since the new PR #6124 implements what added in this PR and on top of that adds the D+D- correlations.
I had left this one open just in case you preferred going step by step

First implementation of D0-D0bar correlation code (data/MC-reco/MC-kine)

46af065

Modified task names

5da5d21

vkucera requested changes Feb 18, 2021

View reviewed changes

Renamed tasks and source file according to conventions

7e324dc

Updates to complying to code conventions

29060c9

vkucera requested changes Feb 19, 2021

View reviewed changes

Updated tasks according to Vit's comments

10bd1e0

vkucera reviewed Mar 1, 2021

View reviewed changes

vkucera requested changes Mar 1, 2021

View reviewed changes

fcolamar added 5 commits March 2, 2021 09:12

Further updates to D0-D0bar correlation task, from code review

d870e9d

Solving conflicts for CMakeLists.txt

23d2c4f

Comply to adaptAnalysisTask core modifications

8ebb50a

Modified label() to mcParticle() call, to comply to PR5566

1ad153b

Clang fix

c4fcec6

fcolamar added 3 commits March 8, 2021 11:29

Merge branch 'dev' into DDbar_FC

9436381

merging with changes from remote dev

Solved conflict in PWGHF/CMakeLists.txt

7b4f941

Merge branch 'dev' into DDbar_FC

94c18e7

vkucera requested changes Mar 8, 2021

View reviewed changes

fcolamar added 3 commits March 9, 2021 14:32

Further round of comments implemented

14eaa5a

Updated CMakeLists.txt due to new task addition

dcf155c

Complying to further adaptAnalysisTask updates

53d2c7a

Missed to update one task in last commit

fc3d699

fcolamar added 2 commits March 18, 2021 12:51

Merge with updated dev

1881b99

Removed the pre-task inserting CollisionId to candidate table (now ad…

9d2330d

…ded centrally)

fcolamar added 11 commits March 25, 2021 09:37

Fixed typo in new version of code

cd41206

Removed cmath header, no longer needed

877a13e

Committing D0-D0bar codes with new structure

5763aa2

Adding definition of task switching options

0e28ede

Clang fixes and task name updates

8c4992f

Major update to D0-D0bar correlation code, split into correlators+ana…

47559b1

…lysis tasks

Implementing dev updates to HFSecondaryVertex.h

4e41fb8

Merge branch 'dev' into DDbar_FC_Restructured

1704562

Merge branch 'DDbar_FC_Restructured' into DDbar_FC

8a49d85

Fix in the branches merging

928e3ea

Clang fix

84645ce

fcolamar mentioned this pull request May 10, 2021

PWGHF: Implementation of D-Dbar generic correlation task and related skimmers (For D0-D0bar and Dplus-Dminus) #6124

Merged

github-actions bot added the stale label May 16, 2021

vkucera closed this May 20, 2021

fcolamar deleted the DDbar_FC branch June 14, 2021 08:40


		Filter filterSelectCandidates = (aod::hf_selcandidate_d0::isSelD0 >= d_selectionFlagD0 \|\| aod::hf_selcandidate_d0::isSelD0bar >= d_selectionFlagD0bar);

		void process(aod::Collision const& collision, soa::Filtered<aod::Big2Prong> const& candidates, aod::BigTracks const& tracks)

		registry.fill(HIST("hDifferenceGenPhiStdPhi"), getDeltaPhi(phi1Std, phi1Gen), pt1);
		registry.fill(HIST("hDifferenceGenPhiByVtxPhi"), getDeltaPhi(phi1ByVtx, phi1Gen), pt1);

		if (cutPtCandMin >= 0. && std::abs(candidate1.pt()) < cutPtCandMin) {
		continue;

Search code, repositories, users, issues, pull requests...

Comments

Conversation

fcolamar commented Feb 11, 2021

Uh oh!

fcolamar commented Feb 11, 2021

Uh oh!

vkucera left a comment

Choose a reason for hiding this comment

Uh oh!

fcolamar commented Feb 18, 2021

Uh oh!

vkucera commented Feb 18, 2021

Uh oh!

ginnocen commented Feb 18, 2021

Uh oh!

vkucera commented Feb 18, 2021

Uh oh!

vkucera commented Feb 19, 2021

Uh oh!

vkucera left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vkucera Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

vkucera Feb 19, 2021 •

edited

Loading

fcolamar Mar 9, 2021 •

edited

Loading