svertexer tuning #12270
svertexer tuning #12270
Conversation
Please consider the following formatting changes to AliceO2Group#12270
| continue; | ||
| } | ||
|
|
||
| if (!hasTPC && nITSclu < mSVParams->mITSSAminNclu) { |
There was a problem hiding this comment.
I thought you wanted to avoid using these tracks in the cascades only. We don't have a problem with V0 finding timing and rejecting on this level may affect K0s efficiency.
Please consider the following formatting changes to AliceO2Group#12270
| } | ||
| if (mV0Hyps[AntiLambda].checkTight(p2Pos, p2Neg, p2V0, ptV0)) { | ||
| goodALamForCascade = true; | ||
| if (ptV0 > mSVParams->minPtV0FromCascade && seedP.nITSclu >= mSVParams->mITSSAminNcluCascades && seedN.nITSclu >= mSVParams->mITSSAminNcluCascades) { |
There was a problem hiding this comment.
But now you are rejecting all tracks with short ITS segment, even if it had TPC. Should not it be
if (ptV0 > mSVParams->minPtV0FromCascade && ((seed.hasTPC && seedP.nITSclu>0 )|| (seedP.nITSclu >= mSVParams->mITSSAminNcluCascades && seedN.nITSclu >= mSVParams->mITSSAminNcluCascades)) {
?
Note that above I also excluded TPC-only tracks in case they are allowed (ping to @f3sch ).
I guess one should add such a check also for the bachelor, e.g. here
AliceO2/Detectors/Vertexing/src/SVertexer.cxx
Line 776 in e927ff6
if (bach.nITSclu<1 || (!baxh.hasTPC && bach.nITSclu<mSVParams->mITSSAminNcluCascades) {
continue;
}
In principle, instead of keeping hasTPC and nClu data-members of the track, we can have a single flag e.g. goodForCascade, set as (hasTPC&&nClu>0) || nClu>=mSVParams->mITSSAminNcluCascades)
There was a problem hiding this comment.
Hi Ruben, you're right, sorry. I changed it now to:
// check tight lambda mass only
bool goodLamForCascade = false, goodALamForCascade = false;
if( ptV0 > mSVParams->minPtV0FromCascade && (seedP.hasTPC || seedP.nITSclu >= mSVParams->mITSSAminNcluCascades) && (seedN.hasTPC || seedN.nITSclu >= mSVParams->mITSSAminNcluCascades) ){
if (mV0Hyps[Lambda].checkTight(p2Pos, p2Neg, p2V0, ptV0) && (!mSVParams->mRequireTPCforCascBaryons || seedP.hasTPC) && seedP.compatibleProton) {
goodLamForCascade = true;
}
if (mV0Hyps[AntiLambda].checkTight(p2Pos, p2Neg, p2V0, ptV0) && (!mSVParams->mRequireTPCforCascBaryons || seedN.hasTPC && seedN.compatibleProton)) {
goodALamForCascade = true;
}
}
and then the bachelor indeed requires:
if( !bach.hasTPC && bach.nITSclu < mSVParams->mITSSAminNcluCascades ){
continue;
}
I'll reply on JIRA too. Thanks!
There was a problem hiding this comment.
One more comment: I deliberately want to avoid a generic flag "is good for cascade" inside the track pool, because in fact, it is at the V0 usage stage that we can decide on applying specific selections on baryons and mesons (because of the bachelor charge), so I would prefer to keep as is if possible. Thanks!
There was a problem hiding this comment.
OK, but still you allow TPC only tracks. @f3sch was going to exclude them but will happen on the same lines you edited, could do at once.
There was a problem hiding this comment.
I will talk to @f3sch later today. Is the idea to have the PCM changes in place also for the Pb-Pb reconstruction? That seems quite challenging to achieve.
Indeed, I did not deliberately reject TPC-only so as to not harm that logical path yet. If we want to do that, sure, we could reject TPC-only with this PR, but if we have a major PR coming up to really tune the TPC-only use, it could also be a part of that to keep the PRs modular. This is a bit what I had in mind. I will write back once I've talked to Felix! Thanks!!
There was a problem hiding this comment.
Hi @shahor02 , so after talking to @f3sch I added a configurable rejection of TPC-only tracks for cascading and for the 3-body decays as well for the sake of the future. We also had a cool discussion on the problem of TPC-only tracks for PCMs :-)
I've now moved this to ready for review. Please check what I did just in case I slipped up, but hopefully we should be good to go now! Thank you and have a nice weekend!
Please consider the following formatting changes to AliceO2Group#12270
Please consider the following formatting changes to AliceO2Group#12270
|
Error while checking build/O2/fullCI for f5432bb at 2023-11-18 12:27: Full log here. |
|
Hi @shahor02 I think the fullCI is unrelated (I guess) but please do let me know in case there's something I should still do! Thanks! |
shahor02
left a comment
shahor02
left a comment
There was a problem hiding this comment.
@ddobrigk the fullCI failure is genuine, and I suspect it is due to not checking if recoData.getITSContributorGID(tvid) returns an ITStracks and not an afterburner tracklet ID, see comment below. Also, yesterday I left a comment about optional switch between (1) and (2) but forgot to push it...
| } | ||
|
|
||
| if (!hasTPC && nITSclu < mSVParams->mITSSAminNclu) { | ||
| continue; // reject short ITS-only |
There was a problem hiding this comment.
@ddobrigk why not doing this rejection at least configurable, to have a possibility to study the difference between 1 and 2?
There was a problem hiding this comment.
@shahor02 what do you mean? If you set mITSSAminNclu to zero you don't cut on anything ;-)
There was a problem hiding this comment.
@shahor02 what we could do if you like is to have an extra option to apply this solely to the cascade finding. It does seem like a bit of overkill to still keep very short and possibly very bad (in quality and especially p reso) ITS tracks, but if you want it, I can add an option, let me know. Thanks! :-D
There was a problem hiding this comment.
I mean to have a possibility to reject short ITS-only tracks in cascades but to accept them in V0.
There was a problem hiding this comment.
what we could do if you like is to have an extra option to apply this solely to the cascade finding. It does seem like a bit of overkill to still keep very short and possibly very bad (in quality and especially p reso) ITS tracks, but if you want it, I can add an option, let me know. Thanks! :-D
Yes, this is what I meant. Even if we don't use this option, it is worth having it evaluate the eff. loss.
| // get Nclusters in the ITS if available | ||
| uint8_t nITSclu = -1; | ||
| auto itsGID = recoData.getITSContributorGID(tvid); | ||
| if (itsGID.isIndexSet() && isITSloaded) { |
There was a problem hiding this comment.
This can be also an ITS-TPC afterburner track, one should change it to
if (itsGID.getSource()==GIndex::ITS && isITSloaded) {
There was a problem hiding this comment.
OK, changed now! Thanks!
Please consider the following formatting changes to AliceO2Group#12270
Please consider the following formatting changes to AliceO2Group#12270
|
Ok so now the |
|
@shahor02 now everything looks fine! Shall we merge? We can then ask Chiara to run a test reco with these latest changes. Thanks a lot! |
3 participants
This draft PR implements the following parameters:
minPtV0FromCascadeallows for a minimum pT for a V0 to be considered for cascading. Default value is 0.3 GeV/c; this is even aggressive since Lambda analysis in pp in Run 2 stopped at 0.4 GeV/c.mITSSAminNcluwhen dealing with tracks with no TPC information (i.e. ITS only), require a minimum number of ITS clusters to have been used in tracking. Default value of 6 to allow only for exceptionally long tracks. (Will have no impact if all sec vertexer sources include TPC)mRequireTPCforCascBaryons: when deciding if a certain V0 is to be considered for cascade finding, require that the V0 prong with charge opposite to the bachelor has TPC signal. This is usually a safe bet because Lambda decays deposit most of the momentum into the proton daughter and we anyway typically want to resort to dE/dx selections of that proton.mFractiondEdxforCascBaryons: in addition to the previous selection, if this parameter is below1.0f, will utilize theo2::tpc::PIDResponseO2 class + a standard set of 2023-averaged BB parameters for checking the compatibility of the V0 baryon daughter with a proton. This parameter, if below1.0f, is the fraction of the expecteddE/dxto be used as threshold. TPC PID in tracking currently uses 6%; therefore, a 6-sigma selection is equivalent to setting this parameter to0.36f.(Tuning still happening, but PR created for discussion already)
Corresponding JIRA discussion: https://alice.its.cern.ch/jira/browse/O2-4382