Uh oh!
There was an error while loading. Please reload this page.
Adding option for tuning dE/dx on data for MC particles - #4733
Adding option for tuning dE/dx on data for MC particles#4733ChSonnabend wants to merge 18 commits into
Conversation
ChSonnabend
commented
Feb 15, 2024
Pinging @jezwilkinson |
Please consider the following formatting changes to AliceO2Group#4733
[MegaLinter] Apply linters automatic fixes to AliceO2Group#4733
Restructuring for TPC TuneOnData
further fixes on table enabling & process switches
jezwilkinson
commented
Feb 20, 2024
Hi @njacazio (also pinging @wiechula in case he wants to take a look), this commit is now ready for review
|
njacazio
commented
Feb 20, 2024
Hi @jezwilkinson all, very good! This is extremely nice, outstanding work! |
wiechula
left a comment
There was a problem hiding this comment.
Thanks a lot @jezwilkinson , I'm not an expert of the analysis framework, so I cannot really give deep comments. A few things I noticed:
- You could consider making variable that don't change
const - There seems to be quite a lot of copy-n-paste code in
pidTPC.cxxandpidTPCFull.cxxis it possible to avoid this? - I might be good to add something about the mc tuned signal to the pid qa, or switch to using the tuned signal in the plots in case it is requested
| if (!trk.hasTPC()) { | ||
| return -999.f; | ||
| } |
There was a problem hiding this comment.
Shouldn't this check logically be the first?
njacazio
left a comment
There was a problem hiding this comment.
Hi all! I like it very much. I'm wondering if in the interest of sanity one should just remove the full and tiny tasks and merge them together and produce only the needed tables. For TOF it would be helpful for sure and I'm wondering if it would make sense here as well, e.g. the dedx table will not be able to be produced if both tiny and full tables are produced.
For the tune on data, did you consider generating directly the nsigmas? It could be easier I guess from the bethe bloch parameters without needing the dedx rewritten.
| // ROOT includes | ||
| #include "TFile.h" | ||
| #include "TRandom.h" |
There was a problem hiding this comment.
One should be careful that the generator does not repeat itself
There was a problem hiding this comment.
added a "gRandom->SetSeed(0)" into the top of the process function, this will reset the seed according to UUID whenever process() is called
| // Parametrization configuration | ||
| bool useCCDBParam = false; | ||
| int getPIDIndex(int pdgCode) |
There was a problem hiding this comment.
Could this go in the PID.h or so?
There was a problem hiding this comment.
Moved the function from both to pidTPCBase.h
| enableFlag("Tr", pidTr); | ||
| enableFlag("He", pidHe); | ||
| enableFlag("Al", pidAl); | ||
| if (doprocessMcTuneOnData) { |
There was a problem hiding this comment.
I would do the opposite, if enabled turn on the process function as well.
There was a problem hiding this comment.
If I'm understanding you right - is it possible to do that? I didn't think the process switches let you change the workflow topology during the init() based on table requirements like that (we would also need to disable the "standard" one in this case) but maybe there's some trick with process switches I didn't know about
There was a problem hiding this comment.
yes, you can tune the process functions based on the needs. That saves quite some overhead.
Something like doprocessMcTuneOnData.value = true; or so, it's very convenient.
jezwilkinson
commented
Feb 22, 2024
Hi @njacazio:
|
njacazio
commented
Feb 22, 2024
Hi @jezwilkinson On the second point, you're right. That works only for the particle of interest, for background it's harder. |
njacazio
commented
Mar 22, 2024
Hi, what is the status of the tune on data? Do you have a delivery estimate? It might be useful for the Pb-Pb |
jezwilkinson
commented
Apr 3, 2024
@njacazio picking back up on this - I made commits a little while ago for the most important points (moving the flag check into the PID header and making sure the seed is explicitly set to the UUID at the top of the process loop so the generator doesn't repeat) |
njacazio
commented
Apr 4, 2024
Hi @jezwilkinson sounds good! |
This PR intends to bring the possibility to enable a flag which runs a separate process function in which a table is filled with dE/dx values sampled randomly from the Bethe-Bloch or Bethe-Bloch + Neural Network corrections. This should be usable similarly to the tune-on-data version from AliPhysics.