TPC PID Response by AnnalenaKa · Pull Request #505 · AliceO2Group/O2Physics

AnnalenaKa · 2022-02-04T10:38:06Z

These changes will move the TPC PID Response into one header file TPCPIDResponse.h and remove any headers and dependencies that are not needed anymore.

njacazio

@AnnalenaKa thanks a lot for the PR and apologies for taking so much time to review it. I have no major comment apart from the need to solve the conflicts that you have to merge, the timestamp dependent call to the CCDB, the Nsigma vs sigma call to fill the table in pidTPC
We can also do it together later if you prefer, let me know

njacazio · 2022-03-07T09:14:03Z

+  } else {
+
+    const double ncl = 159. / track.tpcNClsFound(); //
+    const double p = track.tpcInnerParam();


This should be a float to avoid conversion, this applies also to other variables.

I wanted to use floats for these variables, but the TFormula takes only doubles and no floats. This is why I did not change it.

Ah I understand, ok!
Then we might truly need the c++ implementation, let's see!

njacazio · 2022-03-07T09:24:49Z

-    makeTable(pidHe, tablePIDHe, response, responseHe);
-    makeTable(pidAl, tablePIDAl, response, responseAl);
+    makeTable(pidEl, tablePIDEl, *response, o2::track::PID::Electron);
+    makeTable(pidMu, tablePIDMu, *response, o2::track::PID::Muon);


you could pass *response in the lambda function

njacazio · 2022-03-07T09:27:02Z

        for (auto const& trk : tracks) { // Loop on Tracks
-          const float separation = responsePID.GetSeparation(response, trk);
+          auto collision = collisions.iteratorAt(trk.collisionId());
+          const float separation = response.GetExpectedSigma(collision, trk, pid);


Is this the Nsigma or the expected resolution?

This is the expected resolution. I changed it to the number of sigma. Thanks for spotting this.

Thanks to you!

njacazio · 2022-03-07T09:29:46Z


-  template <o2::track::PID::ID pid>
-  using ResponseImplementation = o2::pid::tpc::ELoss<Trks::iterator, pid>;
  void process(Coll const& collisions, Trks const& tracks)


Here we probably need to merge the timestamp dependent call to the CCDB

Isn't this what we do in line 128, when we get the response?

No, I meant that we we need to align this to the most recent code: https://github.com/AliceO2Group/O2Physics/blob/nightly-20220307/Common/TableProducer/PID/pidTPC.cxx#L140-L146 where the parametrization is taken from the timestamp of the collision

njacazio · 2022-03-07T09:30:15Z

  }
  return workflow;
-}
+}


This might need formatting again

njacazio · 2022-03-07T09:32:02Z

@@ -0,0 +1,253 @@
+// Copyright 2019-2020 CERN and copyright holders of ALICE O2.


For now this is ok, in the future you can consider the implementation of https://github.com/AliceO2Group/O2Physics/blob/nightly-20220307/Common/Tools/handleParamTPCBetheBloch.cxx where the timestamp is taken from the run number to upload the calibration object to the CCDB

njacazio · 2022-03-07T09:32:41Z

-    makeTable(pidHe, tablePIDHe, response, responseHe);
-    makeTable(pidAl, tablePIDAl, response, responseAl);
+    // const o2::pid::tpc::Response& response;
+    makeTable(pidEl, tablePIDEl, *response, o2::track::PID::Electron);


Idem, you can consider passing the *response to the lambda function arguments

…d adjusted for clang format.

…ssed to functions directly - Modified pidTPCFullTest.cxx accordingly - Adjusted for clang-format and checked that it builds

- Adjusted for clang-format

…n added to TPCPIDResponse

njacazio · 2022-03-07T14:36:27Z

Hi @pzhristov we just finished reviewing the code together with @AnnalenaKa and for me it's ok. There is a small downgrade with respect what is currently in the master namely the time dependent parametrization that is taken from the CCDB but we agreed that we will handle this in another PR. I think we can merge this

…eO2Group#594)" This reverts commit 7986c50. This reverts commit 2dc1cfd.

This reverts commit 7986c50. This reverts commit 2dc1cfd.

- Remove old TPC PID response from tasks - Change TPCPIDResponse.h: No template-parameters, particle-id is passed to functions directly - Added loading of TPCPIDResponse-instance from a file - customisable parameters in handleParamTPCResponse, PrintAll() function added to TPCPIDResponse - Add TF1 for Sigma parametrization - use multiplicity tracklets - add extended TPC PIDQA task - add ability to write to CCDB from existing file - add V0 loop to QA task - add eta dependence for TPC V0 QA - Add options default reso param and TFormula param Co-authored-by: Annalena Kalteyer <akalteye@lxir128.gsi.de> Co-authored-by: Jeremy Wilkinson <jeremy.wilkinson@cern.ch> Co-authored-by: Christian Sonnabend <sonnabendch@gmail.com> Co-authored-by: Jeremy Wilkinson <jeremy.wilkinson@gmail.com>

…eO2Group#594)" (AliceO2Group#607) This reverts commit 7986c50. This reverts commit 2dc1cfd.

wiechula suggested changes Feb 16, 2022

View reviewed changes

AnnalenaKa marked this pull request as ready for review February 24, 2022 13:55

AnnalenaKa requested review from a team, alibuild, iarsene, jgrosseo and ktf as code owners February 24, 2022 13:55

njacazio reviewed Mar 7, 2022

View reviewed changes

Annalena Kalteyer and others added 22 commits March 7, 2022 12:24

Remove old TPC PID response from tasks

f36f2f5

Resolve errors

1169ca1

cleanup commit history for TPCPIDResponse

565e414

add missing pidTPCFullTest from previous commit

2569b5a

Added missing parameter mBetheBlochParams[0] in TPCPIDResponse.cxx an…

728f9b6

…d adjusted for clang format.

- Changed TPCPIDResponse.h: No template-parameters, particle-id is pa…

2d31b88

…ssed to functions directly - Modified pidTPCFullTest.cxx accordingly - Adjusted for clang-format and checked that it builds

add TNamed streamer and file generation tool for TPCPIDResponse

be98590

- Added loading of TPCPIDResponse-instance from a file

1beb09e

- Adjusted for clang-format

customisable parameters in handleParamTPCResponse, PrintAll() functio…

ff8bbf5

…n added to TPCPIDResponse

fix whitespace changes

96edbba

cleanup merge messages

32ac0cc

default name of object in handler exec

63e59af

fix missing open-paren

35d0462

remove TNamed inheritance from TPCPIDResponse

8292aca

Clean up tasks including comments Jens

8989bf4

fixing formatting errors

da56a24

fix missing else in mode interpretation

58f500b

Added suggestions of @wiechula

60c00b6

fix default obj path + fallthrough for push mode

2f4c3fe

graceful handling for missing CCDB object in pull mode

0f4ba60

Include Resolition Parametrization

1a83d3a

Add TF1 for Sigma parametrization

9a5d00f

AnnalenaKa and others added 18 commits March 7, 2022 12:29

fix error in resolution calculation

1ec187c

Debug after fixing reso

c911f22

skeleton function to be filled with V0 loop

bca6bb2

add V0 loop to QA task

d5edec6

formatting

c7393e9

fix passing 0 to log scale

58400b3

typo in makeLogaritmic

bf016f2

remove duplications and test versions

a965d60

Modify task to use only default parametrization

1ef2223

Fix clang-formatting

64cdcd1

Add options default reso param and TFormula param

e5320a1

Correct format

554a4ef

remove readout chamber selection

bc31ba2

resolve formatting errors

da8860b

remove qa extended task for now

4ed3763

Correct normalization BBAleph

e4dadfb

Use Nsigma not resolution

3477048

remove response make table

f31ca9e

AnnalenaKa force-pushed the branch_Fixreso branch from 625e115 to f31ca9e Compare March 7, 2022 14:34

Correct use pointer in lambda function

df0813f

njacazio enabled auto-merge (squash) March 8, 2022 09:22

iarsene approved these changes Mar 8, 2022

View reviewed changes

njacazio merged commit 7986c50 into AliceO2Group:master Mar 8, 2022

njacazio added a commit to njacazio/O2Physics that referenced this pull request Mar 11, 2022

Revert "TPC PID Response (AliceO2Group#505)" and "Update TPC QA (Alic…

a1ac725

…eO2Group#594)" This reverts commit 7986c50. This reverts commit 2dc1cfd.

jgrosseo pushed a commit that referenced this pull request Mar 11, 2022

Revert "TPC PID Response (#505)" and "Update TPC QA (#594)" (#607)

4d40aec

This reverts commit 7986c50. This reverts commit 2dc1cfd.

njacazio mentioned this pull request Mar 17, 2022

New TPC PID Response after BugFix #610

Merged

AnnalenaKa deleted the branch_Fixreso branch March 21, 2022 15:27

robincaron13 pushed a commit to robincaron13/O2Physics that referenced this pull request Apr 8, 2022

Revert "TPC PID Response (AliceO2Group#505)" and "Update TPC QA (Alic…

01e04d4

…eO2Group#594)" (AliceO2Group#607) This reverts commit 7986c50. This reverts commit 2dc1cfd.

		@@ -0,0 +1,253 @@
		// Copyright 2019-2020 CERN and copyright holders of ALICE O2.

Conversation

AnnalenaKa commented Feb 4, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

njacazio 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!

njacazio Mar 7, 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!

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!

njacazio commented Mar 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

njacazio Mar 7, 2022 •

edited

Loading