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

reimplementation of Track classifiers and Track Collection mergers #10373

Merged
merged 56 commits into from Jul 29, 2015

Conversation

VinInn
Copy link
Contributor

@VinInn VinInn commented Jul 25, 2015

have fully reimplemented track selectors and merger.
Separated all concerns:
one class to use mva,
one class to use cuts (mimics mva output)
one class to merge selectors
one class to merge collections.
to be done
re-implement duplicate identification and merging
port to HLT

do not esclude that some non-main-stream use-cases are not covered yet.
some more cleanup and optimization is surely required.

I have also implemented a macro to instantiate a sort of DynArray as C++14 does not support anymore
dynamical array.

Full backward compatibility is not maintained.
1)
The vertex identification uses only delta-z and not the track-vertex association:
the latter (besides being utterly slow to verify with the current implementation) it is only working
for Iter0: tracks from all other iterations are not associated to any vertex
2)
quality looseSetWithPV and highPuritySetWithPV is not set (nobody is using it)
and does not make much sense when using MVA anyhow (it can mark if the mva is prompt or detached, maybe. TBD)
3) Iter5 and 6 are MVA only
4) Muon iterations cuts are not identical (and requires further tuning)
5) Duplicate rate a bit higher than before (it needs further tuning anyhow...)
6) JetCore uses a simplified cut-based selection, waiting for its own specific MVA

manually ported from CMSSW_7_5_X #9538 (original by @VinInn).
manual rebase of #10233

@VinInn
Copy link
Contributor Author

VinInn commented Jul 29, 2015

so wrt pre4 in
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_7_6_X_2015-07-24-2300_pr10373/75X_mcRun2_asymptotic_v1_noPU_highPurity/RelValTTbar/effvspos.pdf
we fully recovered the efficiency at large R.
the fake rate is a bit higher and will need some work work.
the efficiency for high-pt jet is higher, fake rate to be retuned.
I think we are going in the right direction

@VinInn
Copy link
Contributor Author

VinInn commented Jul 29, 2015

@davidlange6 , analysis in not really touched

@davidlange6
Copy link
Contributor

Right - but reco signed all of 7 hours ago:)

On Jul 29, 2015, at 8:40 AM, Vincenzo Innocente notifications@github.com wrote:

@davidlange6 , analysis in not really touched


Reply to this email directly or view it on GitHub.

@davidlange6
Copy link
Contributor

@VinInn @slava77 - so the thinking is that DynArray will not be something we want to store as a data format later?

@VinInn
Copy link
Contributor Author

VinInn commented Jul 29, 2015

DynArray is allocated on the stack and does not make sense to store it.
With @makortel , we were thinking to propose to move to FWCore/Utilities a set of "utilities" that may be useful also outside tracker. We will prepare a specific PR for that

davidlange6 added a commit that referenced this pull request Jul 29, 2015
reimplementation of Track classifiers and Track Collection mergers
@davidlange6 davidlange6 merged commit 84d64b1 into cms-sw:CMSSW_7_6_X Jul 29, 2015
@slava77
Copy link
Contributor

slava77 commented Jul 29, 2015

On 7/29/15 2:54 AM, Vincenzo Innocente wrote:

DynArray is allocated on the stack and does not make sense to store it.
With @makortel https://github.com/makortel , we were thinking to
propose to move to FWCore/Utilities a set of "utilities" that may be
useful also outside tracker. We will prepare a specific PR for that

I support this and looking forward for a PR.
The DynArray, to me, is just a more elegant VLA that doesn't disturb
llvm/clang


Reply to this email directly or view it on GitHub
#10373 (comment).

@davidlange6
Copy link
Contributor

@VinInn - I believe this breaks the run1 tracking customize function. Could you update it?

from RecoTracker.IterativeTracking.RunI_iterativeTk_cff import *

File "/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc491/cms/cmssw/CMSSW_7_6_X_2015-07-29-2300/python/RecoTracker/IterativeTracking/RunI_iterativeTk_cff.py", line 16, in
earlyGeneralTracks.selectedTrackQuals[0] = cms.InputTag("initialStepSelector","initialStep")
AttributeError: 'EDProducer' object has no attribute 'selectedTrackQuals'

@VinInn
Copy link
Contributor Author

VinInn commented Jul 30, 2015

nice...
will produce a separate PR, should be easy

@VinInn
Copy link
Contributor Author

VinInn commented Jul 31, 2015

well in 760_pre2 + #10126

25.0_TTbar+TTbarINPUT+DIGI+RECOAlCaCalo+HARVEST+ALCATT]$ cmsDriver.py step3 --datatier GEN-SIM-RECO,DQMIO --conditions auto:run1_mc -s RAW2DIGI,L1Reco,RECO,EI,ALCA:EcalCalZElectron+EcalCalWElectron+EcalUncalZElectron+EcalUncalWElectron+HcalCalIsoTrk,VALIDATION,DQM --eventcontent RECOSIM,DQM -n 100 --filein file:step2.root --fileout file:step3.root --customise RecoTracker/Configuration/customiseForRunI.customiseForRunI

produces

----- Begin Fatal Exception 31-Jul-2015 13:39:55 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing run: 1 lumi: 47 event: 4601
   [1] Running path 'dqmoffline_step'
   [2] Calling event method for module TrackingMonitor/'TrackSeedMonjetCoreRegionalStep'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: std::vector<TrackCandidate>
Looking for module label: jetCoreRegionalStepTrackCandidates
Looking for productInstanceName: 

@VinInn VinInn mentioned this pull request Jul 31, 2015
makortel added a commit to makortel/cmssw that referenced this pull request Oct 2, 2015
…ase1 tracking sequences

The contents of Phase1PU70_preDuplicateMergingGeneralTracks_cfi.py and
Phase1PU70_MuonSeededStep_cff.py are pre-cms-sw#10373.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants