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

era-based run1 tracking setup and T0 Data processing update #13945

Conversation

slava77
Copy link
Contributor

@slava77 slava77 commented Apr 6, 2016

  • Run2_2016_trackingLowPU is added by @makortel to replace functionality earlier available from RecoTracker/Configuration/customiseForRunI.customiseForRunI

DataProcessing setup has a matching ppEra_Run2_2016_trackingLowPU scenario

This PR also includes an attempt to revive the setup with old customization function: it is available with pplowpuEra_Run2_2016 data processing scenario. However, the customization is out of date and while the code configures, it doesn't run.
With the customization, detachedTripletStepChi2Est is a Chi2MeasurementEstimatorESProducer (compared to Chi2ChargeMeasurementEstimatorESProducer with the era). The PSet for the former requires no clusterChargeCut is present and PSet validation fails.

@makortel @VinInn @rovere in case there is some need to support this customization function, it needs an update.
A better choice is to live with the era-based setup.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

A new Pull Request was created by @slava77 (Slava Krutelyov) for CMSSW_8_0_X.

It involves the following packages:

Configuration/DataProcessing
Configuration/StandardSequences
RecoJets/JetProducers
RecoLocalTracker/SiPixelClusterizer
RecoLocalTracker/SiStripClusterizer
RecoMuon/Configuration
RecoPixelVertexing/Configuration
RecoPixelVertexing/PixelTrackFitting
RecoTracker/ConversionSeedGenerators
RecoTracker/FinalTrackSelectors
RecoTracker/IterativeTracking
RecoTracker/TkTrackingRegions
RecoVertex/BeamSpotProducer

@cvuosalo, @cerminar, @cmsbuild, @franzoni, @slava77, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @TaiSakuma, @yslai, @forthommel, @yduhm, @rappoccio, @Martin-Grunewald, @cerati, @threus, @battibass, @makortel, @jhgoh, @jdolen, @jlagram, @OlivierBondu, @trocino, @rociovilar, @abbiendi, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @nhanvtran, @tocheng, @schoef, @mschrode, @dgulhan, @nickmccoll, @gbenelli, @ahinzmann, @dkotlins, @gpetruc, @istaslis, @mariadalfonso, @bachtis 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

@slava77
Copy link
Contributor Author

slava77 commented Apr 6, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

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

@makortel
Copy link
Contributor

makortel commented Apr 6, 2016

@slava77 Did you test if jobs with Run2_2016_trackingLowPU actually run? (just asking because I didn't)

In addition (something I forgot to mention "yesterday" in the e-mails), the DQM customization part of customiseForRunI is not yet migrated to era, because apparently the configuration of tracking DQM has changed since the customize function was crafted such that the customize doesn't do anything for DQM and I need to figure out what exactly needs to be done. I'll work on that but with a lower priority (unless told otherwise), so don't expect DQM (or VALIDATION) to work (yet) with Run2_2016_trackingLowPU.

@slava77 slava77 closed this Apr 6, 2016
@slava77
Copy link
Contributor Author

slava77 commented Apr 6, 2016

submitted too early.
The configs do not really work.

   [1] Running path 'reconstruction_step'
   [2] Calling event method for module SeedGeneratorFromRegionHitsEDProducer/'initialStepSeeds'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: SeedingLayerSetsHits
Looking for module label: initialStepSeedLayers

The test is

cmsDriver.py step3  --conditions auto:run2_data_relval -s RAW2DIGI,L1Reco,RECO\
 --runUnscheduled  --process RECO --data  --eventcontent RECO  --scenario pp --datatier RECO\
 --era Run2_2016_trackingLowPU -n 100\
  --filein /store/data/Run2015B/DoubleEG/RAW/v1/000/251/251/00000/069C1D5D-EA25-E511-8377-02163E013807.root \
 --fileout file:step3.root \
--customise_commands="process.RawToDigi.remove(process.L1TRawToDigi)"

the last line is needed because this is a 2015 input and Stage2 unpackers fail on it; otherwise the data is OK

@makortel feel free to restart from your topic branch and make a PR later today or suggest commits to be added to this PR.

@makortel
Copy link
Contributor

makortel commented Apr 7, 2016

The failure seems to be

An exception of category 'ProductNotFound' occurred while
   [0] Processing run: 251251 lumi: 106 event: 64651630
   [1] Running path 'reconstruction_step'
   [2] Calling event method for module GsfElectronSelector/'electronsWithPresel'
   [3] Calling produce method for unscheduled module GsfElectronEcalDrivenProducer/'ecalDrivenGsfElectrons'
   [4] Calling produce method for unscheduled module GsfElectronCoreEcalDrivenProducer/'ecalDrivenGsfElectronCores'
   [5] Calling produce method for unscheduled module PFElecTkProducer/'pfTrackElec'
   [6] Calling produce method for unscheduled module PFDisplacedTrackerVertexProducer/'pfDisplacedTrackerVertex'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: std::vector<reco::PFDisplacedVertex>
Looking for module label: particleFlowDisplacedVertex
Looking for productInstanceName:

Difficult to see why this would happen only with the Run2_2016_trackingLowPU era. I'll investigate but it may take some time.

As a first observation, if I compare edmConfigDumps of the configuration with Run2_2016 and Run2_2016_trackingLowPU, I see that the particleFlowDisplacedVertex is in the reconstruction_step Path for Run2_2016 but not for Run2_2016_trackingLowPU (it's not the only module that is "missing", e.g. all tracking modules are also "missing"). Could the unscheduled mode play some tricks here?

@makortel
Copy link
Contributor

makortel commented Apr 7, 2016

@slava77 The problem appears to be the same as in https://hypernews.cern.ch/HyperNews/CMS/get/edmFramework/3389.html. In this case, path dqmofflineOnPAT_1_step includes

  • sequence PostDQMOfflineMiniAOD that includes
  • sequence miniAODDQMSequence that includes
  • sequence type0PFMEtCorrectionPFCandToVertexAssociationForValidationMiniAOD that includes
  • particleFlowDisplacedVertex

The simplest fix is to modify customise_commands to be --customise_commands="process.RawToDigi.remove(process.L1TRawToDigi);process.type0PFMEtCorrectionPFCandToVertexAssociationForValidationMiniAOD.remove(process.particleFlowDisplacedVertex)".

More proper fix is edit the DQM configurations (makortel@a3c4b5b), but do we want to do that in this timescale?

I tested both approaches and the recipe #13945 (comment) runs to completion (1000 events).

In the end, I think we would benefit from

  • moving all EDProducers (and EDFilters) out from EndPaths if there are any left
    • and prevent new additions; automatic code to check is actually quite simple (I can provide a base for one)
  • migrating all not-really-event-filter EDFilters to EDProducers

Otherwise I fear we will hit this problem again sooner or later.

@slava77
Copy link
Contributor Author

slava77 commented Apr 7, 2016

On 4/7/16 4:02 AM, Matti Kortelainen wrote:

The failure seems to be

|An exception of category 'ProductNotFound' occurred while [0] Processing
run: 251251 lumi: 106 event: 64651630 [1] Running path
'reconstruction_step' [2] Calling event method for module
GsfElectronSelector/'electronsWithPresel' [3] Calling produce method for
unscheduled module
GsfElectronEcalDrivenProducer/'ecalDrivenGsfElectrons' [4] Calling
produce method for unscheduled module
GsfElectronCoreEcalDrivenProducer/'ecalDrivenGsfElectronCores' [5]
Calling produce method for unscheduled module
PFElecTkProducer/'pfTrackElec' [6] Calling produce method for
unscheduled module
PFDisplacedTrackerVertexProducer/'pfDisplacedTrackerVertex' Exception
Message: Principal::getByToken: Found zero products matching all
criteria Looking for type: std::vectorreco::PFDisplacedVertex Looking
for module label: particleFlowDisplacedVertex Looking for
productInstanceName: |

Difficult to see why this would happen only with the
|Run2_2016_trackingLowPU| era. I'll investigate but it may take some time.

As a first observation, if I compare |edmConfigDump|s of the
configuration with |Run2_2016| and |Run2_2016_trackingLowPU|, I see that
the |particleFlowDisplacedVertex| is in the |reconstruction_step| Path
for |Run2_2016| but not for |Run2_2016_trackingLowPU| (it's not the only
module that is "missing", e.g. all tracking modules are also "missing").
Could the unscheduled mode play some tricks here?

Yes, the unscheduled can have something to do with this.
Since you looked, are these modules completely missing from any path or
do they still show up on another path?
or is it missing completely from the config dump? (less expected)
We've seen the unscheduled conversion lock out some modules in another path,
while still available in the config.
Nominally, if a module is in a config, then getByToken to its product
should trigger this module to run.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#13945 (comment)


Vyacheslav (Slava) Krutelyov
UCSD: 9500 Gillman Dr, Physics UCSD 0354, La Jolla, CA 92093-0354
CERN: 42-R-027
AIM/Skype: siava16 googleTalk: slava77@gmail.com
(630) 291-5128 Cell (US) +41 76 275 7116 Cell (CERN)


@slava77
Copy link
Contributor Author

slava77 commented Apr 7, 2016

@makortel
the fix is in #13411 , but it was not merged yet.
I'll add it on top of my tests and see it if helps

@Dr15Jones
Copy link
Contributor

I checked the configuration generated by the cmsDriver.py command mentioned in the comments and I found

>>> for n,p in process.endpaths.iteritems():
...     if "particleFlowDisplacedVertex" in p.moduleNames():
...             print n
... 
dqmofflineOnPAT_1_step

So the EDProducer is still on the endpath

@Dr15Jones
Copy link
Contributor

If I include the pull request #13979 to this pull request, then the modules 'particleFlowDisplacedVertex' is removed from the endpath 'dqmofflineOnPAT_1_step' and appears on no other endpath.

@slava77
Copy link
Contributor Author

slava77 commented Apr 7, 2016

On 4/7/16 10:18 AM, Chris Jones wrote:

If I include the pull request #13979
#13979 to this pull request, then
the modules 'particleFlowDisplacedVertex' is removed from the endpath
'dqmofflineOnPAT_1_step' and appears on no other endpath.

The #13979 now wors for me.
I messed up with cherry-pick in the last iteration (have to cms-addpkg
or modify sparse-checkout)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#13945 (comment)


Vyacheslav (Slava) Krutelyov
UCSD: 9500 Gillman Dr, Physics UCSD 0354, La Jolla, CA 92093-0354
CERN: 42-R-027
AIM/Skype: siava16 googleTalk: slava77@gmail.com
(630) 291-5128 Cell (US) +41 76 275 7116 Cell (CERN)


@slava77
Copy link
Contributor Author

slava77 commented Apr 7, 2016

here are some comparisons based on the command in #13945 (comment)
red is with trackingLowPU

more generalTracks
all_testlowpuvstestorig_test134p702inera2016c_log10recotracks_generaltracks__reco_obj_pt
tracks are found with the higher/later algos
all_testlowpuvstestorig_test134p702inera2016c_recotracks_generaltracks__reco_obj_algo

more low-pt gsf tracks
all_testlowpuvstestorig_test134p702inera2016c_log10recogsftracks_electrongsftracks__reco_obj_pt

less PF charged hadrons
all_testlowpuvstestorig_test134p702inera2016c_log10recopfcandidates_particleflow__reco_obj_pt15
more so in the barrel
all_testlowpuvstestorig_test134p702inera2016c_recopfcandidates_particleflow__reco_obj_eta12
it's unclear of PF track selections are good for the changes made on the generalTracks side

significantly more conversion tracks
all_testlowpuvstestorig_test134p702inera2016c_log10recotracks_generaltracks__reco_obj_p

For data with B-field and full tracker present things make sense, somewhat.

@makortel
Copy link
Contributor

makortel commented Apr 7, 2016

@slava77 The 81X PR is #13988. It also includes tracking validation for 'trackingLowPU`.

@slava77
Copy link
Contributor Author

slava77 commented Apr 7, 2016

@makortel
and all
I'm waiting for some data from 245200 to be transferred to confirm the behavior on B=0T without pixel

@deguio
Copy link
Contributor

deguio commented Apr 8, 2016

+1

@slava77
Copy link
Contributor Author

slava77 commented Apr 8, 2016

+1

for #13945 0004500

  • jenkins tests pass and comparisons with baseline show no differences as expected for existing workflows
  • tested in CMSSW_8_0_3_patch1
    • see notes above for the effect on complete detector configuration
  • T0 should use ppEra_Run2_2016_trackingLowPU for this purpose (early stable collision period with 0T and no pixel tracker)
  • specific behavior is tested using MinimumBias 245200 file (22C6063B-C7FF-E411-8793-02163E01425D.root ) 2000 events, a run with B=0T and no pixel: the behavior is as expected
    all_ppera_run2_2016_trackinglowpuvsppera_run2_2016_minimumbias245200c_globalmuontracks_pt
    all_ppera_run2_2016_trackinglowpuvsppera_run2_2016_minimumbias245200c_log10recogsftracks_electrongsftracks__reco_obj_pt

all_ppera_run2_2016_trackinglowpuvsppera_run2_2016_minimumbias245200c_log10recotracks_generaltracks__reco_obj_pt

  • short tests were made with DataProcessing scripts as well
python $CMSSW_BASE/src/Configuration/DataProcessing/test/RunExpressProcessing.py --scenario ppEra_Run2_2016_trackingLowPU --fevt --dqmio --global-tag 80X_dataRun2_Express_v4 --lfn=file:22C6063B-C7FF-E411-8793-02163E01425D.root  --alcareco SiStripPCLHistos+SiStripCalZeroBias+SiStripCalMinBias+TkAlMinBias+DtCalib+PromptCalibProd+Hotline+PromptCalibProdSiStrip+PromptCalibProdSiStripGains+LumiPixelsMinBias 

and

python $CMSSW_BASE/src/Configuration/DataProcessing/test/RunPromptReco.py --scenario ppEra_Run2_2016_trackingLowPU --reco --aod --miniaod --dqmio --global-tag 80X_dataRun2_Prompt_v4 --lfn=file:22C6063B-C7FF-E411-8793-02163E01425D.root  --alcareco SiStripCalZeroBias+SiStripCalMinBias+TkAlMinBias  --dqmSeq @common --PhysicsSkim=LogError+LogErrorMonitor

@davidlange6 davidlange6 merged commit f1a09fc into cms-sw:CMSSW_8_0_X Apr 8, 2016
@smuzaffar
Copy link
Contributor

@slava77, the auto forward port (from 80X to 81X) failed for this PR https://cmssdt.cern.ch/jenkins/job/auto-forward-ports/917/console

we need to resolve this.

@makortel
Copy link
Contributor

makortel commented Apr 9, 2016

@smuzaffar The failure is expected, and there is already a partial forward port for 81X (#13988). IMHO it would be easiest if this PR would not be auto-forward-ported.

@davidlt
Copy link
Contributor

davidlt commented Apr 9, 2016

In that case this needs to be merged in CMSSW_8_1_X with '-s ours' to avoid pulling in any changes.

@smuzaffar
Copy link
Contributor

thanks @davidlt

@slava77
Copy link
Contributor Author

slava77 commented Apr 10, 2016

@smuzaffar
What's more practical from your side, modify the ours/theirs flags and re-merge or simply revert the merge in 81X?
If I understand correctly, a revert in 81X would effectively turn off the forward port.

We will catch up with a proper update in 81X shortly (the PR is there, as Matti mentioned already).

Looking at showIB, it's not quite clear what was done. Did you re-merge with '-s ours' ?

@smuzaffar
Copy link
Contributor

@salava77, it is more easy to do the -s ours so that we get this PR out of the way and get other PRs of 80X automatically merged in 81X.

Yes, I did -s ours. If I understand it correctly, it did not actuall merge the changes in 81X.

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