Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ATO-525,ATO-496 - addig qptTgl correction to slove dEdx splitting at pt<0.5 GeV #1319

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

miranov25
Copy link
Contributor

This is first version of the dEdx qPt, tgl correction:

  • Done in the exactly the same way as the PileUp correction
  • In default settings - when switched OFF, validated that it does not influence other corrections
  • Not yet fully tested

@alibuild
Copy link
Collaborator

cf55008: approval required: 1 of @ktf (Giulio Eulisse), @shahor02 (Ruben Shahoyan), @pzhristov (Peter Hristov), @qgp (Jochen Klein), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @sawenzel (Sandro Christian Wenzel), @davidrohr (David Rohr)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@miranov25
Copy link
Contributor Author

I made code modification. Testing results step by step now.
For the LHC15o will be better to apply qPtTgl correction before pileup correction.
I made pull request with modification, but for the moment I only tested that it will not harm, and correction is doing what it should.
I did not test yet full chain.
Mostly reading part

@alibuild
Copy link
Collaborator

cde7ecf: approval required: 1 of @shahor02 (Ruben Shahoyan), @qgp (Jochen Klein), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @davidrohr (David Rohr), @miranov25 (Marian Ivanov); 1 of @ktf (Giulio Eulisse), @shahor02 (Ruben Shahoyan), @pzhristov (Peter Hristov), @qgp (Jochen Klein), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @sawenzel (Sandro Christian Wenzel), @davidrohr (David Rohr)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@alibuild
Copy link
Collaborator

2507d0e: approval required: 1 of @shahor02 (Ruben Shahoyan), @qgp (Jochen Klein), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @davidrohr (David Rohr), @miranov25 (Marian Ivanov); 1 of @ktf (Giulio Eulisse), @shahor02 (Ruben Shahoyan), @pzhristov (Peter Hristov), @qgp (Jochen Klein), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @sawenzel (Sandro Christian Wenzel), @davidrohr (David Rohr)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@chiarazampolli
Copy link
Collaborator

Hello @miranov25 , @wiechula

This is still not merged. Do we need it for the new corrections? Is this relevant only for 15o, so not urgent? Do you plan to do more testing?

Chiara

@miranov25
Copy link
Contributor Author

Hello @wiechula

Did you have time to check the code, or should I commissioned application and reading part?

  • For sure we need the commit
  • It does not affect negatively other code
  • Not yet tested for correction
    • I was waiting for somebody to join hands on session but I did not see enthusiasm within calibration group
    • -> I will have to finish it myself without witnesses

Marian

@alibuild
Copy link
Collaborator

0732116: approval required: 1 of @shahor02 (Ruben Shahoyan), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @qgp (Jochen Klein), @miranov25 (Marian Ivanov); 1 of @shahor02 (Ruben Shahoyan), @davidrohr (David Rohr), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @ktf (Giulio Eulisse), @qgp (Jochen Klein), @pzhristov (Peter Hristov), @sawenzel (Sandro Christian Wenzel)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@pzhristov
Copy link
Collaborator

+test

@alibuild
Copy link
Collaborator

alibuild commented Apr 8, 2021

0732116: testing approved: will not be automatically merged; starting testing. If testing succeeds, merging will require further approval from 1 of @shahor02 (Ruben Shahoyan), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @qgp (Jochen Klein), @miranov25 (Marian Ivanov); 1 of @shahor02 (Ruben Shahoyan), @davidrohr (David Rohr), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @ktf (Giulio Eulisse), @qgp (Jochen Klein), @pzhristov (Peter Hristov), @sawenzel (Sandro Christian Wenzel)

@alibuild
Copy link
Collaborator

alibuild commented Apr 8, 2021

0732116: tests OK, approval required for merging: 1 of @shahor02 (Ruben Shahoyan), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @qgp (Jochen Klein), @miranov25 (Marian Ivanov); 1 of @shahor02 (Ruben Shahoyan), @davidrohr (David Rohr), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @ktf (Giulio Eulisse), @qgp (Jochen Klein), @pzhristov (Peter Hristov), @sawenzel (Sandro Christian Wenzel)

Comment with +1 to merge. Please comment on the pull request: click here and comment at the bottom of the page.

@alibuild
Copy link
Collaborator

Error while checking build/AliRoot/release for 0732116 at 2021-04-11 16:40:

sw/BUILD/AliRoot-latest/log
 80/100 Test  #31: load_library_EMCALTriggerBase ...........   Passed    0.30 sec
        Start  89: load_library_HepMC
 81/100 Test  #98: load_library_TRDgui .....................   Passed    0.29 sec
        Start  12: load_library_lhapdf_5_9_1
 82/100 Test  #77: load_library_MONITOR ....................   Passed    0.30 sec
        Start  86: load_library_EvtGenExternal
 83/100 Test  #93: load_library_AliTPCSpaceChargeBase ......   Passed    0.20 sec
        Start  90: load_library_Photos
 84/100 Test  #11: load_library_lhapdf .....................   Passed    0.17 sec
        Start  17: load_library_pythia6_4_28
 85/100 Test   #9: load_library_HIJING .....................   Passed    0.18 sec
        Start  91: load_library_Tauola
 86/100 Test  #20: load_library_pythia8243 .................   Passed    0.18 sec
        Start  81: load_library_StarLight
 87/100 Test  #15: load_library_pythia6_4_21 ...............   Passed    0.17 sec
        Start  16: load_library_pythia6_4_25
 88/100 Test  #89: load_library_HepMC ......................   Passed    0.18 sec
        Start  87: load_library_EvtGen
 89/100 Test  #12: load_library_lhapdf_5_9_1 ...............   Passed    0.18 sec
        Start  10: load_library_lhapdfbase
 90/100 Test  #86: load_library_EvtGenExternal .............   Passed    0.18 sec
        Start  21: load_library_AMPT
 91/100 Test  #90: load_library_Photos .....................   Passed    0.18 sec
        Start  13: load_library_pythia6
 92/100 Test  #17: load_library_pythia6_4_28 ...............   Passed    0.19 sec
        Start  69: load_library_AliGPUCommon
 93/100 Test  #81: load_library_StarLight ..................   Passed    0.17 sec
 94/100 Test  #91: load_library_Tauola .....................   Passed    0.18 sec
 95/100 Test  #16: load_library_pythia6_4_25 ...............   Passed    0.18 sec
 96/100 Test  #87: load_library_EvtGen .....................   Passed    0.17 sec
 97/100 Test  #10: load_library_lhapdfbase .................   Passed    0.17 sec
 98/100 Test  #21: load_library_AMPT .......................   Passed    0.18 sec
 99/100 Test  #13: load_library_pythia6 ....................   Passed    0.17 sec
100/100 Test  #69: load_library_AliGPUCommon ...............   Passed    0.17 sec

100% tests passed, 0 tests failed out of 100

Total Test time (real) =   7.43 sec
+ cp -v /mnt/mesos/sandbox/sandbox/aliroot-release/sw/BUILD/5a26c800b64a8fa297b4f6139e1a99218bba5786/AliRoot/compile_commands.json /mnt/mesos/sandbox/sandbox/aliroot-release/sw/slc7_x86-64/AliRoot/build_AliRoot_release-1
'/mnt/mesos/sandbox/sandbox/aliroot-release/sw/BUILD/5a26c800b64a8fa297b4f6139e1a99218bba5786/AliRoot/compile_commands.json' -> '/mnt/mesos/sandbox/sandbox/aliroot-release/sw/slc7_x86-64/AliRoot/build_AliRoot_release-1/compile_commands.json'
++ readlink /mnt/mesos/sandbox/sandbox/aliroot-release/sw/SOURCES/AliRoot/build_AliRoot_release/0
+ DEVEL_SOURCES=/mnt/mesos/sandbox/sandbox/aliroot-release/AliRoot
+ [[ /mnt/mesos/sandbox/sandbox/aliroot-release/AliRoot != /mnt/mesos/sandbox/sandbox/aliroot-release/sw/SOURCES/AliRoot/build_AliRoot_release/0 ]]
+ sed -i.deleteme -e 's|/mnt/mesos/sandbox/sandbox/aliroot-release/sw/SOURCES/AliRoot/build_AliRoot_release/0|/mnt/mesos/sandbox/sandbox/aliroot-release/AliRoot|' compile_commands.json
+ rm -f compile_commands.json.deleteme
+ ln -nfs /mnt/mesos/sandbox/sandbox/aliroot-release/sw/BUILD/5a26c800b64a8fa297b4f6139e1a99218bba5786/AliRoot/compile_commands.json /mnt/mesos/sandbox/sandbox/aliroot-release/AliRoot/compile_commands.json
+ rsync -a /mnt/mesos/sandbox/sandbox/aliroot-release/sw/SOURCES/AliRoot/build_AliRoot_release/0/test/ /mnt/mesos/sandbox/sandbox/aliroot-release/sw/slc7_x86-64/AliRoot/build_AliRoot_release-1/test
+ [[ RELWITHDEBINFO == COVERAGE ]]
+ mkdir -p /mnt/mesos/sandbox/sandbox/aliroot-release/sw/slc7_x86-64/AliRoot/build_AliRoot_release-1/etc/modulefiles

Full log here.

@alibuild
Copy link
Collaborator

69e501e: approval required: 1 of @shahor02 (Ruben Shahoyan), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @qgp (Jochen Klein), @miranov25 (Marian Ivanov); 1 of @shahor02 (Ruben Shahoyan), @davidrohr (David Rohr), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @ktf (Giulio Eulisse), @qgp (Jochen Klein), @pzhristov (Peter Hristov), @sawenzel (Sandro Christian Wenzel)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@alibuild
Copy link
Collaborator

aa2f9ec: approval required: 1 of @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @shahor02 (Ruben Shahoyan); 1 of @qgp (Jochen Klein), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @davidrohr (David Rohr), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@alibuild
Copy link
Collaborator

2640966: approval required: 1 of @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @shahor02 (Ruben Shahoyan); 1 of @qgp (Jochen Klein), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @davidrohr (David Rohr), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@shahor02
Copy link
Contributor

+test

@alibuild
Copy link
Collaborator

2640966: testing approved: will not be automatically merged; starting testing. If testing succeeds, merging will require further approval from 1 of @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @shahor02 (Ruben Shahoyan); 1 of @qgp (Jochen Klein), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @davidrohr (David Rohr), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

@alibuild
Copy link
Collaborator

2640966: tests OK, approval required for merging: 1 of @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @shahor02 (Ruben Shahoyan); 1 of @qgp (Jochen Klein), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @davidrohr (David Rohr), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

Comment with +1 to merge. Please comment on the pull request: click here and comment at the bottom of the page.

@alibuild
Copy link
Collaborator

233e219: approval required: 1 of @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @shahor02 (Ruben Shahoyan); 1 of @qgp (Jochen Klein), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @davidrohr (David Rohr), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@alibuild
Copy link
Collaborator

8858b3e: approval required: 1 of @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @shahor02 (Ruben Shahoyan); 1 of @qgp (Jochen Klein), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @davidrohr (David Rohr), @chiarazampolli (Chiara Zampolli), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

Copy link
Contributor

@wiechula wiechula left a comment

Choose a reason for hiding this comment

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

  • There is one problem I see.
  • Also, this does not apply the correction, yet as far as I can see, only allows retrieving it.
  • Also changes in AliPIDResponse will be needed to reflect the changes in the interface here and to be able to switch usage of the qpt correction.

Comment on lines 195 to 196
Bool_t usePileupCorrection = kFALSE,
Bool_t ratio = kFALSE) const;
Bool_t ratio = kFALSE, Bool_t useQPtTglCorrection = kFALSE) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Above you have a different order, first Bool_t useQPtTglCorrection, then Bool_t ratio.

@alibuild
Copy link
Collaborator

5e263cd: approval required: 1 of @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @shahor02 (Ruben Shahoyan); 1 of @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @qgp (Jochen Klein), @chiarazampolli (Chiara Zampolli), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@alibuild
Copy link
Collaborator

49e34a1: approval required: 1 of @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @shahor02 (Ruben Shahoyan); 1 of @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @qgp (Jochen Klein), @chiarazampolli (Chiara Zampolli), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@miranov25
Copy link
Contributor Author

Hello @wiechula

I fixed some problems, now I am including modification for the high dEdx correction. Please, do not approve yet.

Regards
Marian

Copy link
Contributor

@wiechula wiechula left a comment

Choose a reason for hiding this comment

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

Only very minor comments.
Still, the modifications in AliPIDResponse need to be done.

@@ -765,7 +773,7 @@ Float_t AliTPCPIDResponse::GetSignalDelta(const AliVTrack* track,
ETPCdEdxSource dedxSource,
Bool_t correctEta,
Bool_t correctMultiplicity,
Bool_t usePileupCorrection /*= kFALSE*/,
Bool_t usePileupCorrection /*= kFALSE*/, Bool_t useQPtTglCorrection,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Bool_t usePileupCorrection /*= kFALSE*/, Bool_t useQPtTglCorrection,
Bool_t usePileupCorrection /*= kFALSE*/,
Bool_t useQPtTglCorrection,

@@ -173,27 +173,27 @@ class AliTPCPIDResponse: public TNamed {
ETPCdEdxSource dedxSource = kdEdxDefault,
Bool_t correctEta = kFALSE,
Bool_t correctMultiplicity = kFALSE,
Bool_t usePileupCorrection = kFALSE) const;
Bool_t usePileupCorrection = kFALSE, Bool_t useQPtTglCorrection = kFALSE) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Bool_t usePileupCorrection = kFALSE, Bool_t useQPtTglCorrection = kFALSE) const;
Bool_t usePileupCorrection = kFALSE,
Bool_t useQPtTglCorrection = kFALSE) const;

Double_t GetExpectedSigma( const AliVTrack* track,
AliPID::EParticleType species,
ETPCdEdxSource dedxSource = kdEdxDefault,
Bool_t correctEta = kFALSE,
Bool_t correctMultiplicity = kFALSE,
Bool_t usePileupCorrection = kFALSE) const;
Bool_t usePileupCorrection = kFALSE, Bool_t useQPtTglCorrection = kFALSE) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Bool_t usePileupCorrection = kFALSE, Bool_t useQPtTglCorrection = kFALSE) const;
Bool_t usePileupCorrection = kFALSE,
Bool_t useQPtTglCorrection = kFALSE) const;

Float_t GetNumberOfSigmas( const AliVTrack* track,
AliPID::EParticleType species,
ETPCdEdxSource dedxSource = kdEdxDefault,
Bool_t correctEta = kFALSE,
Bool_t correctMultiplicity = kFALSE,
Bool_t usePileupCorrection = kFALSE) const;
Bool_t usePileupCorrection = kFALSE, Bool_t useQPtTglCorrection = kFALSE) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Bool_t usePileupCorrection = kFALSE, Bool_t useQPtTglCorrection = kFALSE) const;
Bool_t usePileupCorrection = kFALSE,
Bool_t useQPtTglCorrection = kFALSE) const;


Float_t GetSignalDelta( const AliVTrack* track,
AliPID::EParticleType species,
ETPCdEdxSource dedxSource = kdEdxDefault,
Bool_t correctEta = kFALSE,
Bool_t correctMultiplicity = kFALSE,
Bool_t usePileupCorrection = kFALSE,
Bool_t ratio = kFALSE) const;
Bool_t useQPtTglCorrection = kFALSE, Bool_t ratio = kFALSE) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Bool_t useQPtTglCorrection = kFALSE, Bool_t ratio = kFALSE) const;
Bool_t useQPtTglCorrection = kFALSE,
Bool_t ratio = kFALSE) const;

@@ -318,7 +323,7 @@ class AliTPCPIDResponse: public TNamed {
const TSpline3* responseFunction,
Bool_t correctEta,
Bool_t correctMultiplicity,
Bool_t usePileupCorrection) const;
Bool_t usePileupCorrection, Bool_t useQPtTglCorrection = kFALSE) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Bool_t usePileupCorrection, Bool_t useQPtTglCorrection = kFALSE) const;
Bool_t usePileupCorrection,
Bool_t useQPtTglCorrection = kFALSE) const;

@@ -328,7 +333,7 @@ class AliTPCPIDResponse: public TNamed {
const TSpline3* responseFunction,
Bool_t correctEta,
Bool_t correctMultiplicity,
Bool_t usePileupCorrection) const;
Bool_t usePileupCorrection, Bool_t useQPtTglCorrection = kFALSE) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Bool_t usePileupCorrection, Bool_t useQPtTglCorrection = kFALSE) const;
Bool_t usePileupCorrection,
Bool_t useQPtTglCorrection = kFALSE) const;

@alibuild
Copy link
Collaborator

033a34f: approval required: 1 of @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @shahor02 (Ruben Shahoyan); 1 of @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @qgp (Jochen Klein), @chiarazampolli (Chiara Zampolli), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@miranov25
Copy link
Contributor Author

Hello @wiechula

I changed the formatting in header filed and i added static functions to correct momenta and dEdx for energy loss within TPC
I did not commit yet usage of that correction. Maybe we can meet, to specify how users will speicyfy parameters needed.
Free parameters:

  • gas type (Argon, Neon)
  • E loss scaling i respect to the nominal tabulated value for that gases ...

@alibuild
Copy link
Collaborator

e140dc5: approval required: 1 of @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @shahor02 (Ruben Shahoyan); 1 of @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @qgp (Jochen Klein), @chiarazampolli (Chiara Zampolli), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@miranov25
Copy link
Contributor Author

Hello @wiechula

I added energy loss correction added. Please do not merge yet. I have to check full chain.

…loating exception

stress test at GSI needed ...
…loating exception

code crashed if the BG<0.005 - at very low momenta ...
for Mesut code crashed because not check of the momentum range
stress/crash  test at GSI to RUN
@alibuild
Copy link
Collaborator

c8238b3: approval required: 1 of @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @qgp (Jochen Klein), @miranov25 (Marian Ivanov), @shahor02 (Ruben Shahoyan); 1 of @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @ktf (Giulio Eulisse), @pzhristov (Peter Hristov), @qgp (Jochen Klein), @chiarazampolli (Chiara Zampolli), @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@miranov25
Copy link
Contributor Author

Fixing problem seen in Mesut jobs

  • bbAleph not well defined below BG 0.05 - it could be even negative - putting to the code protection against outliers

Comment on lines +2461 to +2483
double AliTPCPIDResponse::GetPtOutHelix(Float_t ptIn, Double_t tgl, Float_t mass, Double_t rIn, Double_t rOut, Int_t type, Float_t bz,Float_t fraction){
const Float_t kB2C=-0.299792458e-3;
const Float_t kMaxSnp=0.95;
Float_t pin=ptIn*TMath::Sqrt(1+tgl*tgl);
Float_t qPt=1/ptIn;
Float_t bg = pin/mass;
Float_t elossNe = 0.001*0.00156; // page 3 https://www.slac.stanford.edu/pubs/icfa/summer98/paper3/paper3.pdf
Float_t elossAr = 0.001*0.00244; // page 3 https://www.slac.stanford.edu/pubs/icfa/summer98/paper3/paper3.pdf
Float_t dEdx= AliExternalTrackParam::BetheBlochAleph(bg)*fraction;
Double_t e0=TMath::Sqrt(pin*pin+mass*mass);
Float_t eloss=(type==0) ? elossAr:elossNe;
Float_t lengthNorm=TMath::Abs(rOut-rIn)*TMath::Sqrt(1+tgl*tgl);
Float_t C = kB2C*bz*qPt;
if (TMath::Abs(0.5*rIn*C)>1) rIn=2.*kMaxSnp/C;
if (TMath::Abs(0.5*rOut*C)>1) rOut=2.*kMaxSnp/C;
Float_t lIn = 2.*TMath::ASin(0.5*rIn*C)/C;
Float_t lOut = 2.*TMath::ASin((0.5*rOut*C))/C;
lengthNorm=(TMath::Abs(lOut)-TMath::Abs(lIn))*TMath::Sqrt(1+tgl*tgl);
//
Double_t e1=e0-dEdx*eloss*lengthNorm;
if (e1<mass) return 0;
return TMath::Sqrt(e1*e1-mass*mass)/TMath::Sqrt(1+tgl*tgl);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Why mix float and double precision?
  • Is double needed? If not I propose to make all float otherwise all double.
  • nearly all values can be const.
  • Why is lengthNorm overwritten? The first value is not used.

/// \param bz - magnetic field (kGaus)
/// \param nStep - number of steps for Euler integration
/// \return
double AliTPCPIDResponse::dEdxRationR0R1Helix(Float_t ptIn, Float_t tgl, Float_t mass, Double_t rIn0,Float_t rOut0, Double_t rIn1,Float_t rOut1, Int_t type, Float_t bz, Int_t nStep){
Copy link
Contributor

Choose a reason for hiding this comment

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

Also in this function, why mit float and double. Make all variables const which don't change.

/// \param nStep - Euler integration approximation with n steps
/// \param useInnerPt - default kFLASE - used for debugging purposes
/// \return
double AliTPCPIDResponse::dEdxMeanToInHelix(Float_t ptIn, Float_t tgl, Float_t mass, Double_t rIn,Float_t rOut, Int_t type, Float_t bz, Int_t nStep, Float_t scale, Bool_t useInnerPt){
Copy link
Contributor

Choose a reason for hiding this comment

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

Also in this function, why mit float and double. Make all variables const which don't change.

@miranov25
Copy link
Contributor Author

  • For the functions - they were needed for the TTreeFormula evaluation
  • TTreeFormula are evaluating (sending them) as doubles
  • I did not pay attention to mix double/float but in some calculation like sqrt(A2-B2) the double precision is preferable to avoid numerical instabilities
    -- I can change it
  • For the const vs normal I do not care as argument - I am not C++ fundametatlist. Variables are local _> function does not change stat in outer scope
    -- I can change it
    I suggest to meet to make modification on zoom
    For the moment I make changes which are needed to simplify testing

@alibuild
Copy link
Collaborator

2f249b4: approval required: 1 of @miranov25 (Marian Ivanov), @shahor02 (Ruben Shahoyan), @davidrohr (David Rohr), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @qgp (Jochen Klein); 1 of @sawenzel (Sandro Christian Wenzel), @shahor02 (Ruben Shahoyan), @ktf (Giulio Eulisse), @davidrohr (David Rohr), @qgp (Jochen Klein), @jgrosseo (Jan Fiete Grosse-Oetringhaus), @chiarazampolli (Chiara Zampolli), @pzhristov (Peter Hristov)

Comment with +1 to approve and allow automatic merging,or with +test to run tests only. Please comment on the pull request: click here and comment at the bottom of the page.

@miranov25
Copy link
Contributor Author

Commit above - check in tree loading (check type of the key using dynamic cast) 2f249b4
Please do not merge yet

@alibuild
Copy link
Collaborator

alibuild commented Apr 27, 2022

Error while checking build/AliRoot/macos for 2f249b4 at 2022-09-28 22:16:

No log files found

Full log here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants