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

MT MTV (and speedup) #4669

Merged
merged 13 commits into from Jul 21, 2014
Merged

MT MTV (and speedup) #4669

merged 13 commits into from Jul 21, 2014

Conversation

VinInn
Copy link
Contributor

@VinInn VinInn commented Jul 16, 2014

This PR contains two independent set of changes:

  1. declare as stream modules a bunch of producers related to Tracker reconstruction and validation
  2. dramatically improve the computational performance of the MultiTrackValidator

for this last one (together with #4655) MTV at high pileup speeds up by a factor 5.

tested. no regression observed.

NB in MT mode the MTV cannot be use din standalone mode yet...

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @VinInn (Vincenzo Innocente) for CMSSW_7_2_X.

Mt mtv

It involves the following packages:

CommonTools/RecoAlgos
RecoLocalTracker/SiPixelRecHits
RecoLocalTracker/SiStripRecHitConverter
SimDataFormats/TrackingAnalysis
SimTracker/Common
SimTracker/TrackerHitAssociation
Validation/RecoTrack

@civanch, @ojeda, @StoyanStoynev, @danduggan, @rovere, @cmsbuild, @nclopezo, @mdhildreth, @deguio, @slava77, @Degano can you please review it and eventually sign? Thanks.
@GiacomoSguazzoni, @rovere, @dkotlins, @wmtford, @gpetruc, @cerati, @threus, @venturia this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@VinInn VinInn changed the title Mt mtv MT MTV (and speedup) Jul 16, 2014
@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@civanch
Copy link
Contributor

civanch commented Jul 21, 2014

+1

@deguio
Copy link
Contributor

deguio commented Jul 21, 2014

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine).

ktf added a commit that referenced this pull request Jul 21, 2014
@ktf ktf merged commit 48f05fc into cms-sw:CMSSW_7_2_X Jul 21, 2014
@Martin-Grunewald
Copy link
Contributor

Hi Vincenzo,

Did you by any chance remove check on a product handle?

I see this error which was not present yesterday, after which your PR was integrated....
This is in a HIon workflow.

%MSG
----- Begin Fatal Exception 22-Jul-2014 09:09:33 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing run: 1 lumi: 6 event: 26
   [1] Running path 'validation_step'
   [2] Calling event method for module MultiTrackValidator/'hiTrackValidator'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for a container with elements of type: reco::Track
Looking for module label: generalTracks
Looking for productInstanceName:

   Additional Info:
      [a] If you wish to continue processing events after a ProductNotFound exc\
eption,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSe\
t in the configuration.

----- End Fatal Exception -------------------------------------------------

@Martin-Grunewald
Copy link
Contributor

It must be this PR as my test works in CMSSW_7_2_X_2014-07-21-1400
while it crashes as shown above in CMSSW_7_2_X_2014-07-22-0200,
and the only PR integrated between these two IBs is this one...

@VinInn
Copy link
Contributor Author

VinInn commented Jul 22, 2014

On 22 Jul, 2014, at 9:58 AM, Martin Grunewald notifications@github.com wrote:

Hi Vincenzo,

Did you by any chance remove check on a product handle?
Not explicitely.
this one is still there
https://github.com/VinInn/cmssw/blob/19eb65ad622162638a9b50dcf8105e2148cc7d3b/Validation/RecoTrack/plugins/MultiTrackValidator.cc#L259
and this other one was never protected explicitely
https://github.com/VinInn/cmssw/blob/19eb65ad622162638a9b50dcf8105e2148cc7d3b/Validation/RecoTrack/plugins/MultiTrackValidator.cc#L250
unless is not an issue with consume and not may consume here
https://github.com/VinInn/cmssw/blob/19eb65ad622162638a9b50dcf8105e2148cc7d3b/Validation/RecoTrack/src/MultiTrackValidatorBase.cc#L19

maybe Hi should set ignoremissingtrackcollection ?
(how could happen that a collection is missing in just one envent?)

I see this error which was not present yesterday, after which your PR was integrated....
This is in a HIon workflow.

%MSG
----- Begin Fatal Exception 22-Jul-2014 09:09:33 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
[0] Processing run: 1 lumi: 6 event: 26
[1] Running path 'validation_step'
[2] Calling event method for module MultiTrackValidator/'hiTrackValidator'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for a container with elements of type: reco::Track
Looking for module label: generalTracks
Looking for productInstanceName:

(how can I reproduce it? I can run under gdb and find the line number…)
I hate these exceptions w/o trace back, an assert would be infiniitely more useful
v.

@Martin-Grunewald
Copy link
Contributor

To reproduce in a developer area using today's 2Am IB:

cd src
git cms-addpkg HLTrigger/Configuration
scram build -j 4
cd HLTrigger/Configuration/test/
./runAll.csh HIon

The last cmd creates and runs a series of test jobs leading up to the failing one.

I belive the collection was actually never created (and was never accessed as ther was no
crash in the past?) but now it is in some event...??

@Martin-Grunewald
Copy link
Contributor

Based on the collection label, I believe it is this one:

trackCollectionForDrCalculation

@VinInn
Copy link
Contributor Author

VinInn commented Jul 22, 2014

which was supposed to fail?
Finish ./runIntegration.csh DATA HIon

Finished ./runOne.csh DATA HIon

Resulting log files:
-rw-r--r--. 1 innocent zh 529 Jul 22 10:49 HLT_Integration_HIon_DATA.log
-rw-r--r--. 1 innocent zh 427 Jul 22 10:58 HLT_Integration_HIon_STARTUP.log
-rw-r--r--. 1 innocent zh 140517 Jul 22 10:53 IntegrationTestWithHLT_cfg.log
-rw-r--r--. 1 innocent zh 52871 Jul 22 10:45 ONLINE_HLT_HIon_DATA.log
-rw-r--r--. 1 innocent zh 53206 Jul 22 10:47 ONLINE_HLT_HIon_STARTUP.log
-rw-r--r--. 1 innocent zh 62568 Jul 22 10:57 RelVal_DigiL1RawHLT_HIon_STARTUP.log
-rw-r--r--. 1 innocent zh 48957 Jul 22 10:46 RelVal_DigiL1Raw_HIon_STARTUP.log
-rw-r--r--. 1 innocent zh 47064 Jul 22 10:46 RelVal_HLT2_HIon_DATA.log
-rw-r--r--. 1 innocent zh 47276 Jul 22 10:47 RelVal_HLT2_HIon_STARTUP.log
-rw-r--r--. 1 innocent zh 47136 Jul 22 10:45 RelVal_HLT_HIon_DATA.log
-rw-r--r--. 1 innocent zh 47337 Jul 22 10:47 RelVal_HLT_HIon_STARTUP.log
-rw-r--r--. 1 innocent zh 85293 Jul 22 10:47 RelVal_HLT_Reco_HIon_DATA.log
-rw-r--r--. 1 innocent zh 85471 Jul 22 10:54 RelVal_HLT_Reco_HIon_STARTUP.log
-rw-r--r--. 1 innocent zh 37340 Jul 22 10:44 RelVal_L1RePack_HIon_DATA.log
-rw-r--r--. 1 innocent zh 117895 Jul 22 10:48 RelVal_RECO_HIon_DATA.log
-rw-r--r--. 1 innocent zh 103797 Jul 22 10:58 RelVal_RECO_HIon_STARTUP.log
[innocent@vinavx2 test]$ pwd
/home/vin/CMSSW_7_2_X_2014-07-22-0200/src/HLTrigger/Configuration/test
[innocent@vinavx2 test]$ echo $CMSSW_BASE/
/home/vin/CMSSW_7_2_X_2014-07-22-0200/
[innocent@vinavx2 test]$ echo $CMSSW_RELEASE_BASE/
/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc481/cms/cmssw-patch/CMSSW_7_2_X_2014-07-22-0200/
[innocent@vinavx2 test]$

@VinInn
Copy link
Contributor Author

VinInn commented Jul 22, 2014

"trackCollectionForDrCalculation" yes, this is our suspicion too.
In principle also HLTmultiTrackValidator_cff.py should crash.
since a while thug as I cannot find a check that trackCollectionForDrCalculation is valid even in the old code (besides that it is not declared in the consume neither)

@Martin-Grunewald
Copy link
Contributor

RelVal_RECO_HIon_STARTUP should fail...

@VinInn
Copy link
Contributor Author

VinInn commented Jul 22, 2014

to say the truth I do not understand how it was working before given

# setup multi-track validator
from Validation.RecoTrack.MultiTrackValidator_cff import *
hiTrackValidator = multiTrackValidator.clone(
    label_tp_effic = cms.InputTag("primaryChgSimTracks"),
    label_tp_fake  = cms.InputTag("cutsTPFake"),
    signalOnlyTP = cms.bool(False),
    skipHistoFit = cms.untracked.bool(True), # done in post-processing
    minpT = cms.double(1.0),
    maxpT = cms.double(100.0),
    nintpT = cms.int32(40),
    useLogPt = cms.untracked.bool(True)
    )

hiTrackValidator.label = cms.VInputTag(cms.InputTag('cutsRecoTracks'),
                                       cms.InputTag('cutsRecoTracksHP')
                                       )

and given

   ### for fake rate vs dR ###
    trackCollectionForDrCalculation = cms.InputTag("generalTracks")
)

in
Validation/RecoTrack/python/MultiTrackValidator_cfi.py

and why the consume does not complains that "generalTracks" is not there?

@Martin-Grunewald
Copy link
Contributor

Hmm, the consumes call itself can not complain as it is done at construction time
(at most a check on an empty label in the inputtag?)
The error can only be caught at runtime, if after the getByToken the Handle is
invalid because the product does not exist (ie, getByToken returns false).

@VinInn
Copy link
Contributor Author

VinInn commented Jul 22, 2014

git cms-merge-topic rovere:fix_HI_MTV_for_Dr
fix it
will push soon.
Our opinion is that it was broken since a while.
@mtosi , could you please check hltMTV? should suffer the same illness...

@VinInn VinInn deleted the MT_MTV branch July 13, 2016 13:47
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

9 participants