svertexer: adaptive mass windows #12136
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shahor02
requested changes
Oct 24, 2023
| float getMargin(float pt) const | ||
| { | ||
| if( mPIDV0 == PID::XiMinus || mPIDV0 == PID::OmegaMinus) { | ||
| if( mPars[NSigmaM] * getSigmaCascade(pt) + mPars[MarginM] > 0.006 ) return mPars[NSigmaM] * 0.006; |
Collaborator
There was a problem hiding this comment.
Please use {}, even for one-liners.
Can we calculate getSigmaCascade only once?
Also, the SigmaCascade is used both for V0s and Cascades, which is somewhat misleading. Perhaps the name should be changed?
On top of that: the mPIDV0 == PID::Lambda will use getSigmaCascade but PID::AntiLambda will use ggetSigma. Not sure this is intended.
Finally, what is the difference in the final mass interval defined using getSigmaCascade vs tuned getSigma?
If this is just a few MeV, I would go for a simpler and faster pT resolution param.
There was a problem hiding this comment.
Actually, under PID::Lambda, also AntiLambda is included. the last case is left for HyperTriton and Hyperhydrog4 as I didn't want to change anything not valid for cascades or V0s.
I adjusted the names and added some comments, so after the new commit, it should be clear how it was meant.
Collaborator
There was a problem hiding this comment.
For the Lambda vs AntiLambda, indeed, confused it with HypV0:.., where these are different enums, sorry.
| if( mPIDV0 == PID::XiMinus || mPIDV0 == PID::OmegaMinus) { | ||
| if( mPars[NSigmaM] * getSigmaCascade(pt) + mPars[MarginM] > 0.006 ) return mPars[NSigmaM] * 0.006; | ||
| return mPars[NSigmaM] * getSigmaCascade(pt) + mPars[MarginM]; | ||
| }else if( mPIDV0 == PID::K0 || mPIDV0 == PID::Lambda ) return mPars[NSigmaM] * getSigmaCascade(pt) + mPars[MarginM]; |
Please consider the following formatting changes to AliceO2Group#12136
shahor02
reviewed
Oct 26, 2023
| float pidCutsLambda[SVertexHypothesis::NPIDParams] = {0., 20, 0., 1.09004e-03, 2.62291e-04, 8.93179e-03, 2.83121}; // Lambda | ||
| float pidCutsHTriton[SVertexHypothesis::NPIDParams] = {0.0025, 14, 0.07, 0.5, 0.0, 0.0, 0.0}; // HyperTriton | ||
| float pidCutsHhydrog4[SVertexHypothesis::NPIDParams] = {0.0025, 14, 0.07, 0.5, 0.0, 0.0, 0.0}; // Hyperhydrog4 - Need to update | ||
| float pidCutsPhoton[SVertexHypothesis::NPIDParams] = {0.001, 20, 0.60, 20, 0.60, 0.0, 0.0, 0.0, 0.0}; // Photon |
Collaborator
There was a problem hiding this comment.
is it normal to have 600 MeV margin on the photon (old setting)?
Contributor
Author
There was a problem hiding this comment.
This is a very good question but I did not touch it. I think the problem is that we currently apply logical OR to 7 types of mass hypotheses, so if one of them has very open cuts, the whole concept of windows will not be particularly helpful. I guess this is a question more to the PWG-EM conveners... Do we know where the 600 MeV/c2 comes from in the first place?
Collaborator
There was a problem hiding this comment.
No but @f3sch is now studying conversions, perhaps he can extract realistic reconstructed mass range for photons
| float pidCutsHTriton[SVertexHypothesis::NPIDParams] = {0.0025, 14, 0.07, 0.5, 0.0, 0.0, 0.0}; // HyperTriton | ||
| float pidCutsHhydrog4[SVertexHypothesis::NPIDParams] = {0.0025, 14, 0.07, 0.5, 0.0, 0.0, 0.0}; // Hyperhydrog4 - Need to update | ||
| float pidCutsPhoton[SVertexHypothesis::NPIDParams] = {0.001, 20, 0.60, 20, 0.60, 0.0, 0.0, 0.0, 0.0}; // Photon | ||
| float pidCutsK0[SVertexHypothesis::NPIDParams] = {0., 20, 0., 4.0, 0.0, 2.84798e-03, 9.84206e-04, 3.31951e-03, 2.39438}; // K0 |
Collaborator
There was a problem hiding this comment.
and not to have any margin on K0s?
Contributor
Author
There was a problem hiding this comment.
The current function has a minimum now, but we could add a margin of a couple of MeV/c2 if we feel like it (and it would not have a significant impact)... I think the 600 MeV/c2 is more worrisome, though :-)
| if (mV0Hyps[ipid].check(p2Pos, p2Neg, p2V0, ptV0)) { | ||
| goodHyp = hypCheckStatus[ipid] = true; | ||
| } | ||
| if (mV0Hyps[ipid].checkTight(p2Pos, p2Neg, p2V0, ptV0)) { |
Collaborator
There was a problem hiding this comment.
I assume the "tight" check is always tighter than non-tight one, in this case, this check can go inside the if of non-tight one.
Also, why do you need to perform the tight check for anything but the Lambda and AntiLambda?
BTW, the 3-body also need tight selection, right? Also, currently it selects in a custom way
if (massForLambdaHyp - mV0Hyps[ipid].getMassV0Hyp() < mV0Hyps[ipid].getMargin(ptV0)) {
Should not one use abs of the difference at the
AliceO2/Detectors/Vertexing/src/SVertexer.cxx
Line 578 in 9e3673d
mV0Hyps[ipid].checkTight(p2Pos, p2Neg, p2V0, ptV0) ?Pinging to @fmazzasc and @mpuccio
Also, it is possible to use proper IDs instead of 2-4 at https://github.com/AliceO2Group/AliceO2/blob/9e3673d6e4c3edeae9a718600ff6132d288d824e/Detectors/Vertexing/src/SVertexer.cxx#L576C2-L576C33
Contributor
There was a problem hiding this comment.
Hi @shahor02, we need an asymmetric cut to cover the phase space of the 3-body decays. Hypernuclei do not decay in a Lambda but to Nucleus+p+pi. We can change the ids.
Collaborator
Collaborator
|
Hi @ddobrigk Current selection With this PR: I see that the expected Lambda width with new parametrization is 1.8MeV above 1 GeV, isn't just 4 sigma cut too tight? |
Contributor
I think 1.8 MeV are in line with what Chiara presented last week: see sl 14 https://indico.cern.ch/event/1326916/contributions/5583405/attachments/2734727/4755042/ALICEWeek_StrangenessRun3.pdf compared with the Lucia's parametrisation How many Collisions do we have here? |
Contributor
Author
|
Hi @shahor02 , traditional analyses of cascades in Pb-Pb have typically used a fixed mass window for Lambdas that is around 5-6MeV/c2 (see e.g. https://alice-notes.web.cern.ch/system/files/notes/analysis/1384/2023-10-04-AnalysisNote_PbPb5.pdf). Therefore, having a 4-sigma window is actually OK and making it pT-dependent is even better than what was done before. If in doubt we could also increase that to 5-sigma. In your numbers, in addition to knowing how many events this is for your printout (as Francesco asked), I would like to know: is the "current" selection the case in which we apply absolutely no mass window? In that case these numbers aren't really representative of anything and I would not worry; what is more important is actually that we have a negligible efficiency cost when switching. In that sense, the best is to generate a tiny batch of cascade enriched MC and redo the svertexer part with and without this new selection and check that the efficiency is minimally altered before we go on. I have some scripts for that and am playing around with that now. Thanks a lot! |
Collaborator
|
@ddobrigk missed your comment, sorry for the late reply. As I wrote, @davidrohr 's test was for 25kHz pbpb, so there were ~70 collisions per TF.
|
Contributor
Author
|
The latest version does the following:
On a different note, I am not 100% sure the base V0 mass window for stored V0 candidates is actually being applied correctly now, but I did not touch this with this commit. I will proceed to cross-check this more to be sure - it might be that because of the logical OR with the 7 hypotheses not much gets cut out? I am not entirely sure. |
Contributor
Author
Ah - sorry, now it was me who did not 100% follow through with all the consequences of these numbers. In fact, you're right - the fact that the windows are not so effective at reducing the volume (especially of the V0s) is already there, as you say. I suggest we do the following:
|
Collaborator
@ddobrigk Will test it tomorrow, but my concern with the previous version of this PR (tested by @davidrohr) was that too few cascades survive. |
Contributor
Author
Yes, this was a bug I now fixed :-) you should get a good number of cascades now! |
Collaborator
|
@ddobrigk Now for the same TF I get to be compared with: no mass selection at all and "current mass selection": Concerning photons: @f3sch sent me the histos showing that |
Collaborator
|
I think we can start merging this PR, could you please fix the clang-format and remove the |
Please consider the following formatting changes to AliceO2Group#12136
shahor02
approved these changes
Nov 2, 2023
noferini
pushed a commit
that referenced
this pull request
Nov 3, 2023
* svertexer: adaptive mass windows * One more step: Lucia's changes * Tighter windows when doing decay chains * Please consider the following formatting changes * Latest bugfixes * Please consider the following formatting changes --------- Co-authored-by: David Dobrigkeit Chinellato <david.dobrigkeit.chinellato.cern.ch> Co-authored-by: ALICE Action Bot <alibuild@cern.ch>
christianreckziegel
pushed a commit
to LucasFerrandi/AliceO2
that referenced
this pull request
Nov 9, 2023
* svertexer: adaptive mass windows * One more step: Lucia's changes * Tighter windows when doing decay chains * Please consider the following formatting changes * Latest bugfixes * Please consider the following formatting changes --------- Co-authored-by: David Dobrigkeit Chinellato <david.dobrigkeit.chinellato.cern.ch> Co-authored-by: ALICE Action Bot <alibuild@cern.ch>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is Lucia's work to parametrize V0 and cascade masses in a more accurate way. Some final testing is still ongoing. Note also that, as before, parameters are hardcoded; it may pay off to make them settable so a minor commit to do that might still come later today. More soon...