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

Add GsfTracks to tracking validation #13348

Merged
merged 9 commits into from Mar 2, 2016

Conversation

cmsbuild
Copy link
Contributor

This PR adds the monitoring of GsfTracks (electronGsfTracks collection) to tracking validation. The efficiency is defined wrt. TrackingParticles with abs(pdgId) == 11, and fake rate wrt. all TrackingParticles. On the same go, the definition of fake rate for conversions is also changed to be wrt. all TrackingParticles. In order to achieve these nicely, and to again properly support the separate label_tp_effic and label_tp_fake parameters of MultiTrackValidator, the following changes were made

  • The configuration of TrackingParticleNumberOfLayersProducer is fixed
  • MTV supports reading TrackingParticleRefVector instead of TrackingParticleCollection
    • It would have been easier to just use View<TrackingParticle>, but TrackToTrackingParticleAssociator interface does not support it, and changing it would have had much larger impact. (I'll probably try to do it anyway some day)
  • TrackingParticleConversionSelector is changed to produce TrackingParticleRefVector (and renamed and turned to edm::global)
  • TrackValidation_cffis migrated to use TrackingParticleRefVector instead of copies
  • SimToReco (TP->track) association is created wrt. label_tp_fake TPs instead of label_tp_eff, and label_tp_eff is required to be a subset of label_tp_fake (explicit check is added)
    • The SimToReco association is used in efficiency numerator, but also in duplicate rate numerator. For duplicates, the SimToReco must contain associations for TPs from label_tp_fake. For efficiency, these "extra" associations don't matter since the associations are checked only for label_tp_eff.

Tested in CMSSW_8_0_X_2016-02-15-1100 (with and without #13242), expecting changes under Tracking/TrackConversion (for fakes and other track-based histograms), and addition of Tracking/TrackGsf folder.

@rovere @VinInn
Automatically ported from CMSSW_8_0_X #13324 (original by @makortel).

…or instead of TrackingParticleCollection

Something like this is necessary to be able to have separate sets of
TrackingParticles for efficiency and fake rate plots. It would be
easier to just use View<TrackingParticle>, but that would have
non-trivial impact on the track-TP associator/association interfaces,
so it is left to later time.
Duplicates are defined as "track associated to TP associated to > 1
tracks". The first association is done with recoToSim, and the second
with simToReco. If the efficiency and fake TP sets are different, this
leads to (unexpected) effects in duplicate. Naively one would assume
that the duplicate definition would be independet of the set of TPs
used for efficiencies.

The fix is to do the simToReco association wrt. the fake TPs. For
efficiency histograms it doesn't matter if the simToReco has entries
for TPs not in efficiency TP set.
@cmsbuild
Copy link
Contributor Author

A new Pull Request was created by @cmsbuild for CMSSW_8_1_X.

It involves the following packages:

CommonTools/RecoAlgos
SimGeneral/TrackingAnalysis
Validation/RecoTrack

@civanch, @cvuosalo, @mdhildreth, @cmsbuild, @deguio, @slava77, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@rappoccio, @jdolen, @battibass, @makortel, @abbiendi, @GiacomoSguazzoni, @jhgoh, @VinInn, @ahinzmann, @istaslis, @rovere, @wmtford, @cerati, @trocino, @dgulhan this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@cvuosalo
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor Author

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/11357/console

@cmsbuild
Copy link
Contributor Author

-1
Tested at: 5eb3f94
When I ran the RelVals I found an error in the following worklfows:
5.1 step1

runTheMatrix-results/5.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log

135.4 step1

runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13348/11357/summary.html

@slava77
Copy link
Contributor

slava77 commented Feb 19, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor Author

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/11374/console

@cmsbuild
Copy link
Contributor Author

@cvuosalo
Copy link
Contributor

+1

For #13324 5eb3f94

Adding GsfTracks to tracking validation. #13324 is the 80X version of this PR, and it has already been approved by Reco.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-02-18-2300 show only the differences expected for this PR, with examples shown in the #13324 thread.

@civanch
Copy link
Contributor

civanch commented Feb 19, 2016

+1

@makortel
Copy link
Contributor

ping DQM (@deguio, @vanbesien)

@slava77
Copy link
Contributor

slava77 commented Feb 26, 2016

ping DQM (@deguio, @vanbesien)
your signature is needed.
Thank you.

@deguio
Copy link
Contributor

deguio commented Mar 1, 2016

+1

@cmsbuild
Copy link
Contributor Author

cmsbuild commented Mar 1, 2016

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Mar 2, 2016
Add GsfTracks to tracking validation
@cmsbuild cmsbuild merged commit d59dbdb into cms-sw:CMSSW_8_1_X Mar 2, 2016
@davidlange6
Copy link
Contributor

@makortel - we have some crashes from this PR. For example:
https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc6_amd64_gcc493/CMSSW_8_1_X_2016-03-02-2300/pyRelValMatrixLogs/run/140.1_QCD_Pt_80_120_13_HI+QCD_Pt_80_120_13_HIINPUT+DIGIHI+RECOHI+HARVESTHI/step3_QCD_Pt_80_120_13_HI+QCD_Pt_80_120_13_HIINPUT+DIGIHI+RECOHI+HARVESTHI.log

An exception of category 'InvalidReference' occurred while
[0] Processing run: 1 lumi: 1 event: 2
[1] Running path 'prevalidation_step'
[2] Calling event method for module MultiTrackValidator/'trackValidatorConversion'
Exception Message:
ClusterTPAssociation has TrackingParticles with ProductID 2:142, but got TrackingParticleRef/Handle/ProductID with ID 0:0. This is typically caused by a configuration error.
----- End Fatal Exception -------------------------------------------------
Another exception was caught while trying to clean up files after the primary fatal exception.

@makortel
Copy link
Contributor

makortel commented Mar 3, 2016

@davidlange6 I noticed them too and have already a fix #13570. I'll provide a fix for the failures in HI workflows separately (they are also caused by this PR).

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

7 participants