Femtoworld by zchochul · Pull Request #1186 · AliceO2Group/O2Physics

zchochul · 2022-09-02T07:35:44Z

Changed printfs and couts for LOGFs and LOGs
Fixing filling phi candidates table filling

alibuild · 2022-09-02T08:14:31Z

Error while checking build/O2Physics/o2 for fc72792 at 2022-09-02 10:14:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/1186-slc7_x86-64/0/PWGCF/FemtoWorld/TableProducer/femtoWorldProducerTask.cxx:385:12: error: variable 'cutContainer' set but not used [-Werror=unused-but-set-variable]
/sw/SOURCES/O2Physics/1186-slc7_x86-64/0/PWGCF/FemtoWorld/TableProducer/femtoWorldProducerTask.cxx:583:15: error: unused variable 'indexChildID' [-Werror=unused-variable]
/sw/SOURCES/O2Physics/1186-slc7_x86-64/0/PWGCF/FemtoWorld/TableProducer/femtoWorldProducerTask.cxx:375:9: error: variable 'childIDs' set but not used [-Werror=unused-but-set-variable]
ninja: build stopped: subcommand failed.

Full log here.

alibuild · 2022-09-02T14:44:42Z

Error while checking build/O2Physics/o2 for 5e282bb at 2022-09-04 17:23:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/1186-slc7_x86-64/0/PWGCF/FemtoWorld/TableProducer/femtoWorldProducerTask.cxx:385:12: error: variable 'cutContainer' set but not used [-Werror=unused-but-set-variable]
/sw/SOURCES/O2Physics/1186-slc7_x86-64/0/PWGCF/FemtoWorld/TableProducer/femtoWorldProducerTask.cxx:584:15: error: unused variable 'indexChildID' [-Werror=unused-variable]
ninja: build stopped: subcommand failed.

Full log here.

victor-gonzalez · 2022-09-05T14:49:12Z

PWGCF/FemtoWorld/Core/FemtoWorldCollisionSelection.h

  void printCuts()
  {
-    std::cout << "Debug information for FemtoWorldCollisionSelection \n Max. z-vertex: " << mZvtxMax << "\n Check trigger: " << mCheckTrigger << "\n Trigger: " << mTrigger << "\n Check offline: " << mCheckOffline << "\n";
+    // std::cout << "Debug information for FemtoWorldCollisionSelection \n Max. z-vertex: " << mZvtxMax << "\n Check trigger: " << mCheckTrigger << "\n Trigger: " << mTrigger << "\n Check offline: " << mCheckOffline << "\n";


No comments left, please

victor-gonzalez · 2022-09-05T14:50:44Z

PWGCF/FemtoWorld/Core/FemtoWorldDetaDphiStar.h

+      /*for (int i = 0; i < 2; i++) {
+        std::string dirName = static_cast<std::string>(dirNames[1]);
+        histdetadpi[i][0] = mHistogramRegistry->add<TH2>((dirName + static_cast<std::string>(histNames[0][i])).c_str(), "; #Delta #eta; #Delta #phi", kTH2F, {{100, -0.15, 0.15}, {100, -0.15, 0.15}});
+        histdetadpi[i][1] = mHistogramRegistry->add<TH2>((dirName + static_cast<std::string>(histNames[1][i])).c_str(), "; #Delta #eta; #Delta #phi", kTH2F, {{100, -0.15, 0.15}, {100, -0.15, 0.15}});
+        if (plotForEveryRadii) {
+          for (int j = 0; j < 9; j++) {
+            histdetadpiRadii[i][j] = mHistogramRegistryQA->add<TH2>((dirName + static_cast<std::string>(histNamesRadii[i][j])).c_str(), "; #Delta #eta; #Delta #phi", kTH2F, {{100, -0.15, 0.15}, {100, -0.15, 0.15}});
+          }
+        }
+      }*/


Please remove comments
You can test your code locally until you are happy with it and then push it but don't leave pending code commented

victor-gonzalez · 2022-09-05T14:51:23Z

PWGCF/FemtoWorld/Core/FemtoWorldDetaDphiStar.h

+      /*
+       if (part1.partType() != o2::aod::femtoworldparticle::ParticleType::kTrack || part2.partType() != o2::aod::femtoworldparticle::ParticleType::kV0) {
+         LOG(fatal) << "FemtoWorldDetaDphiStar: passed arguments don't agree with FemtoWorldDetaDphiStar instantiation! Please provide kTrack,kV0 candidates.";
+         return false;
+       }
+
+       bool pass = false;
+       for (int i = 0; i < 2; i++) {
+         auto indexOfDaughter = part2.index() - 2 + i;
+         auto daughter = particles.begin() + indexOfDaughter;
+         auto deta = part1.eta() - daughter.eta();
+         auto dphiAvg = AveragePhiStar(part1, *daughter, i);
+         histdetadpi[i][0]->Fill(deta, dphiAvg);
+         if (pow(dphiAvg, 2) / pow(deltaPhiMax, 2) + pow(deta, 2) / pow(deltaEtaMax, 2) < 1.) {
+           pass = true;
+         } else {
+           histdetadpi[i][1]->Fill(deta, dphiAvg);
+         }
+       }
+       */


Same comment as above

victor-gonzalez · 2022-09-05T14:53:26Z

PWGCF/FemtoWorld/Core/FemtoWorldPhiSelection.h

-    printf("-Something wrong in isSelectedMinimal--\n");
-    printf("ERROR - Wrong sign for Phi daughters\n");
+    LOGF(info, "-Something wrong in isSelectedMinimal--\n");
+    LOGF(info, "ERROR - Wrong sign for Phi daughters\n");


Consider using

LOGF(error, "....);

and consider using a single line with all error information
Be aware that you can use formatting the same way you do it with printf

victor-gonzalez · 2022-09-05T14:55:18Z

PWGCF/FemtoWorld/Core/FemtoWorldPhiSelection.h

+    LOGF(info, "-Something wrong in isSelectedMinimal--\n");
+    LOGF(info, "ERROR - Wrong sign for Phi daughters\n");


Same as above

victor-gonzalez · 2022-09-05T14:55:53Z

PWGCF/FemtoWorld/Core/FemtoWorldPhiSelection.h

+    // float phiPx = posTrack.px() + negTrack.px();
+    // float phiPy = posTrack.py() + negTrack.py();
+    // float phiPz = posTrack.pz() + negTrack.pz();


Please, don't leave code commented

victor-gonzalez · 2022-09-05T14:56:21Z

PWGCF/FemtoWorld/Core/FemtoWorldV0Selection.h

+    LOGF(info, "-Something wrong in isSelectedMinimal--\n");
+    LOGF(info, "ERROR - Wrong sign for V0 daughters\n");


Same comment as above

victor-gonzalez · 2022-09-05T14:56:40Z

PWGCF/FemtoWorld/Core/FemtoWorldV0Selection.h

+    LOGF(info, "-Something wrong in isSelectedMinimal--\n");
+    LOGF(info, "ERROR - Wrong sign for V0 daughters\n");


Same comments as above

victor-gonzalez · 2022-09-05T14:57:07Z

PWGCF/FemtoWorld/Tasks/CMakeLists.txt

+#o2physics_add_dpl_workflow(femto-world-pair-track-v0
+#          SOURCES femtoWorldPairTaskTrackV0.cxx
+#          PUBLIC_LINK_LIBRARIES O2::Framework O2Physics::AnalysisCore
+#          COMPONENT_NAME Analysis)


No comments left

PWGCF/FemtoWorld/Tasks/femtoWorldPairTaskTrackTrack.cxx

victor-gonzalez

Please, fix the formatting errors and implement the suggested changes

alibuild · 2022-09-05T15:51:39Z

Error while checking build/O2Physics/o2 for 5ca42fc at 2022-09-05 17:51:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/1186-slc7_x86-64/0/PWGCF/FemtoWorld/Tasks/femtoWorldPairTaskTrackPhi.cxx:130:17: error: unused variable 'magFieldTesla' [-Werror=unused-variable]
ninja: build stopped: subcommand failed.

Full log here.

alibuild · 2022-09-06T09:27:23Z

Error while checking build/O2Physics/o2 for 90a7419 at 2022-09-06 11:27:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/1186-slc7_x86-64/0/PWGCF/FemtoWorld/Tasks/femtoWorldPairTaskTrackPhi.cxx:130:17: error: unused variable 'magFieldTesla' [-Werror=unused-variable]
ninja: build stopped: subcommand failed.

Full log here.

…for now)

alibuild · 2022-09-09T13:04:45Z

Error while checking build/O2Physics/o2 for 07582c1 at 2022-09-09 15:04:

## sw/BUILD/O2Physics-latest/log
/sw/SOURCES/O2Physics/1186-slc7_x86-64/0/PWGCF/FemtoWorld/Tasks/femtoWorldPairTaskTrackPhi.cxx:130:17: error: unused variable 'magFieldTesla' [-Werror=unused-variable]
ninja: build stopped: subcommand failed.

Full log here.

PWGCF/FemtoWorld/Core/FemtoWorldContainer.h

victor-gonzalez

Please, get rid of all commented code
If you are requiring to merge you code it will go into production
If you are just keeping your changes in a central repository change your PR to draft

PWGCF/FemtoWorld/Core/FemtoWorldContainer.h

victor-gonzalez · 2022-09-09T14:04:29Z

PWGCF/FemtoWorld/DataModel/FemtoWorldDerived.h

+// DECLARE_SOA_DYNAMIC_COLUMN(P, p, //! Compute the overall momentum in GeV/c
+//                            [](float pt, float eta) -> float {
+//                              return pt * std::cosh(eta);
+//                            });
+//  debug variables


Please, no commented code

victor-gonzalez · 2022-09-09T14:05:15Z

PWGCF/FemtoWorld/DataModel/FemtoWorldDerived.h

                  femtoworldparticle::Py<femtoworldparticle::Pt, femtoworldparticle::Phi>,
                  femtoworldparticle::Pz<femtoworldparticle::Pt, femtoworldparticle::Eta>,
-                  femtoworldparticle::P<femtoworldparticle::Pt, femtoworldparticle::Eta>, // tu koniec poprzedniej wersji
+                  // femtoworldparticle::P<femtoworldparticle::Pt, femtoworldparticle::Eta>, // tu koniec poprzedniej wersji


Please, no commented code

victor-gonzalez · 2022-09-09T14:05:55Z

PWGCF/FemtoWorld/TableProducer/femtoWorldProducerTask.cxx

+                                 aod::pidTPCKa, aod::pidTPCPr, aod::pidTPCDe,
+                                 aod::pidTOFEl, aod::pidTOFMu, aod::pidTOFPi,
+                                 aod::pidTOFKa, aod::pidTOFPr, aod::pidTOFDe, aod::pidTOFbeta>;
 // using FilteredFullV0s = soa::Filtered<aod::V0Datas>; /// predefined Join table for o2::aod::V0s = soa::Join<o2::aod::TransientV0s, o2::aod::StoredV0s> to be used when we add v0Filter


Please, no commented code

victor-gonzalez · 2022-09-09T14:06:30Z

PWGCF/FemtoWorld/TableProducer/femtoWorldProducerTask.cxx

      trackCuts.fillQA<aod::femtoworldparticle::ParticleType::kTrack, aod::femtoworldparticle::TrackType::kNoChild>(track);
      // the bit-wise container of the systematic variations is obtained
-      auto cutContainer = trackCuts.getCutContainer<aod::femtoworldparticle::cutContainerType>(track);
+      // auto cutContainer = trackCuts.getCutContainer<aod::femtoworldparticle::cutContainerType>(track);


Please, no commented code

PWGCF/FemtoWorld/TableProducer/femtoWorldProducerTask.cxx

victor-gonzalez · 2022-09-09T14:14:49Z

PWGCF/FemtoWorld/TableProducer/femtoWorldProducerTask.cxx

    }
-  }
-  PROCESS_SWITCH(femtoWorldProducerTask, processPhi, "Produce Phi candidates tables", true);
+  */}


Same as above

PWGCF/FemtoWorld/Tasks/femtoWorldPairTaskTrackPhi.cxx

victor-gonzalez · 2022-09-09T14:16:51Z

PWGCF/FemtoWorld/Tasks/femtoWorldPairTaskTrackPhi.cxx

+      /*
      if (p1.p() > cfgCutTable->get("PartOne", "MaxP") || p1.pt() > cfgCutTable->get("PartOne", "MaxPt")) {
        continue;
      }
+
      if (!isFullPIDSelected(p1.pidcut(), p1.p(), cfgCutTable->get("PartOne", "PIDthr"), vPIDPartOne, cfgNspecies, kNsigma, cfgCutTable->get("PartOne", "nSigmaTPC"), cfgCutTable->get("PartOne", "nSigmaTPCTOF"))) {
        continue;
      }


Same as above

This was commented only temporarily. In next PR I will be adding cuts to the histograms

victor-gonzalez

I still see files with code commented
This is not a good practice for going into production
Commenting code for making it readable is OK. Leaving code commented is not, it makes difficult its interpretation
For the time being I approve the PR to move it forward. But please, spend some time in removing all commented lines of code

zchochul · 2022-09-13T09:32:27Z

I'll try to avoid leaving commented code. Thank you for understanding!

zchochul requested review from alibuild, jgrosseo, saganatt and victor-gonzalez as code owners September 2, 2022 07:35

zchochul force-pushed the femtoworld branch from 15232d6 to fc72792 Compare September 2, 2022 07:47

victor-gonzalez reviewed Sep 5, 2022

View reviewed changes

PWGCF/FemtoWorld/Tasks/femtoWorldPairTaskTrackTrack.cxx Outdated Show resolved Hide resolved

victor-gonzalez requested changes Sep 5, 2022

View reviewed changes

zchochul added 6 commits September 9, 2022 14:41

Fixing couts and printfs and filling phi candidates tables

8c910d3

fixing clang, small progress with phi producer

e20c47c

Changes to the producer

85ea1fe

Phi continuation

52d8ed1

Changes to the producer

7b93786

Phi candidates' histograms filling up properly (but without PID cuts …

acdd62f

…for now)

zchochul force-pushed the femtoworld branch from 6568e17 to acdd62f Compare September 9, 2022 12:42

adding changes requested by victor-gonzalez

07582c1

victor-gonzalez reviewed Sep 9, 2022

View reviewed changes

PWGCF/FemtoWorld/Core/FemtoWorldContainer.h Outdated Show resolved Hide resolved

victor-gonzalez requested changes Sep 9, 2022

View reviewed changes

zchochul added 3 commits September 9, 2022 16:40

Getting rid of the comments

b0cb9a3

Getting rid of the comments part 2

b601bd6

Fixing trailing spaces at the end of the line

0cc7a14

zchochul requested a review from victor-gonzalez September 9, 2022 16:31

zchochul added 2 commits September 12, 2022 15:19

Added cuts for protons

51800fe

Invariant mass cuts

87306a3

victor-gonzalez approved these changes Sep 13, 2022

View reviewed changes

victor-gonzalez merged commit 8b99c96 into AliceO2Group:master Sep 13, 2022

		LOGF(info, "-Something wrong in isSelectedMinimal--\n");
		LOGF(info, "ERROR - Wrong sign for Phi daughters\n");

		LOGF(info, "-Something wrong in isSelectedMinimal--\n");
		LOGF(info, "ERROR - Wrong sign for V0 daughters\n");

Conversation

zchochul commented Sep 2, 2022

Uh oh!

alibuild commented Sep 2, 2022

Uh oh!

alibuild commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

victor-gonzalez Sep 5, 2022 • 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!

Uh oh!

victor-gonzalez left a comment

Choose a reason for hiding this comment

Uh oh!

alibuild commented Sep 5, 2022

Uh oh!

alibuild commented Sep 6, 2022

Uh oh!

alibuild commented Sep 9, 2022

Uh oh!

Uh oh!

victor-gonzalez left a comment

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

victor-gonzalez left a comment

Choose a reason for hiding this comment

Uh oh!

zchochul commented Sep 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

alibuild commented Sep 2, 2022 •

edited

Loading

victor-gonzalez Sep 5, 2022 •

edited

Loading