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

Enable mkFit in InitialStepPreSplitting,InitialStep,HighPtTripletStep,DetachedQuadStep tracking iterations for phase1 pixel era, except for HI and special 2017 tracking eras #35457

Merged
merged 5 commits into from Oct 5, 2021

Conversation

makortel
Copy link
Contributor

@makortel makortel commented Sep 28, 2021

PR description:

This PR enables mkFit by default in the following 4 tracking iterations

  • InitialStepPreSplitting
  • InitialStep
  • HighPtTripletStep
  • DetachedQuadStep

for the entire era of phase 1 pixel detector (i.e. from 2017 to end of Run 3), except for HI eras (Run2_2017_pp_on_XeXe, Run2_2017_ppRef, Run2_2018_pp_on_AA, Run2_2018_pp_on_AA_noHCALmitigation, Run3_pp_on_PbPb) and 2017 special tracking eras (Run2_2017_trackingLowPU, Run2_2017_trackingRun2), following the plan presented e.g. in https://indico.cern.ch/event/1071296/#1-news .

This PR also adds Run3_noMkFit Era to easily use Run3 era without mkFit.

Technically this PR makes use of the existing per-iteration Modifiers in order to avoid conflicts with concurrent developments to enable mkFit in additional iterations. Those are planned to be cleaned up soon, after the development settles.

PR validation:

Checked locally that both 11634.0 and 11634.7 run, and from their printouts that mkFit is used in initialStep.

Expecting no changes in 11634.7. Expecing changes in all (other) phase 1 pixel workflows (except those eras listed above) that are along those shown in https://indico.cern.ch/event/992975/?showDate=all&showSession=all#8-mkfit-status-and-plans.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35457/25612

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

  • Configuration/Eras (operations)
  • Configuration/StandardSequences (operations)

@perrotta, @cmsbuild, @qliphy, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@fabiocos, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @Martin-Grunewald, @missirol, @lecriste, @mtosi, @ebrondol, @mmusich, @dgulhan, @slomeo this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

…,DetachedQuadStep for phase1Pixel, except for HI and Run2_2017_{trackingLowPU,trackingRun2}

Done with new trackingMkFitProd Era ModifierChain, to avoid confusion
with the current 'trackingMkFit' ProcessModifier ModifierChain that is
being used as part of development. To be cleaned up after the
development settles.
@makortel makortel changed the title Enable mkFit in InitialStepPreSplitting,InitialStep,HighPtTripletStep,DetachedQuadStep tracking iterations for phase1 pixel era Enable mkFit in InitialStepPreSplitting,InitialStep,HighPtTripletStep,DetachedQuadStep tracking iterations for phase1 pixel era, except for HI and special 2017 tracking eras Sep 28, 2021
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35457/25614

@cmsbuild
Copy link
Contributor

Pull request #35457 was updated. @perrotta, @cmsbuild, @qliphy, @fabiocos, @davidlange6 can you please check and sign again.

@makortel
Copy link
Contributor Author

test parameters:

  • workflows = 148.0,149.0,140.56,159.0,10024.3,10024.4

@makortel
Copy link
Contributor Author

@cmsbuild, please test

@makortel
Copy link
Contributor Author

assign reconstruction

@cmsbuild
Copy link
Contributor

New categories assigned: reconstruction

@slava77,@jpata you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ddcd2/19212/summary.html
COMMIT: a5c4336
CMSSW: CMSSW_12_1_X_2021-09-28-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35457/19212/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test TestDQMOnlineClient-pixel_dqm_sourceclient had ERRORS
---> test TestConfigDP had ERRORS

AddOn Tests

----- Begin Fatal Exception 28-Sep-2021 22:03:01 CEST-----------------------
An exception of category 'LogicError' occurred while
   [0] Processing  Event run: 323775 lumi: 99 event: 111372621 stream: 3
   [1] Running path 'FEVTDEBUGHLToutput_step'
   [2] Prefetching for module PoolOutputModule/'FEVTDEBUGHLToutput'
   [3] Prefetching for module ReducedRecHitCollectionProducer/'reducedEcalRecHitsEB'
   [4] Prefetching for module InterestingDetIdCollectionProducer/'interestingEcalDetIdPFEB'
   [5] Prefetching for module PFECALSuperClusterProducer/'particleFlowSuperClusterECAL'
   [6] Prefetching for module RecoChargedRefCandidatePrimaryVertexSorter/'offlinePrimaryVertices'
   [7] Prefetching for module ChargedRefCandidateProducer/'trackRefsForJetsBeforeSorting'
   [8] Prefetching for module TrackWithVertexRefSelector/'trackWithVertexRefSelectorBeforeSorting'
   [9] Prefetching for module DuplicateListMerger/'generalTracks'
   [10] Prefetching for module TrackProducer/'mergedDuplicateTracks'
   [11] Prefetching for module DuplicateTrackMerger/'duplicateTrackCandidates'
   [12] Prefetching for module TrackCollectionMerger/'preDuplicateMergingGeneralTracks'
   [13] Prefetching for module TrackCollectionMerger/'earlyGeneralTracks'
   [14] Prefetching for module TrackProducer/'highPtTripletStepTracks'
   [15] Prefetching for module MkFitOutputConverter/'highPtTripletStepTrackCandidates'
   [16] Calling method for module MkFitProducer/'highPtTripletStepTrackCandidatesMkFit'
Exception Message:
MkFitHitWrapper has pixel cluster ID 0:0 but pixel cluster mask has 4:897
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 28-Sep-2021 21:58:30 CEST-----------------------
An exception of category 'LogicError' occurred while
   [0] Processing  Event run: 323775 lumi: 99 event: 111372621 stream: 2
   [1] Running path 'FEVTDEBUGHLToutput_step'
   [2] Prefetching for module PoolOutputModule/'FEVTDEBUGHLToutput'
   [3] Prefetching for module ReducedRecHitCollectionProducer/'reducedEcalRecHitsEB'
   [4] Prefetching for module InterestingDetIdCollectionProducer/'interestingEcalDetIdPFEB'
   [5] Prefetching for module PFECALSuperClusterProducer/'particleFlowSuperClusterECAL'
   [6] Prefetching for module RecoChargedRefCandidatePrimaryVertexSorter/'offlinePrimaryVertices'
   [7] Prefetching for module ChargedRefCandidateProducer/'trackRefsForJetsBeforeSorting'
   [8] Prefetching for module TrackWithVertexRefSelector/'trackWithVertexRefSelectorBeforeSorting'
   [9] Prefetching for module DuplicateListMerger/'generalTracks'
   [10] Prefetching for module TrackProducer/'mergedDuplicateTracks'
   [11] Prefetching for module DuplicateTrackMerger/'duplicateTrackCandidates'
   [12] Prefetching for module TrackCollectionMerger/'preDuplicateMergingGeneralTracks'
   [13] Prefetching for module TrackCollectionMerger/'earlyGeneralTracks'
   [14] Prefetching for module TrackProducer/'highPtTripletStepTracks'
   [15] Prefetching for module MkFitOutputConverter/'highPtTripletStepTrackCandidates'
   [16] Calling method for module MkFitProducer/'highPtTripletStepTrackCandidatesMkFit'
Exception Message:
MkFitHitWrapper has pixel cluster ID 0:0 but pixel cluster mask has 4:347
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 28-Sep-2021 22:00:45 CEST-----------------------
An exception of category 'LogicError' occurred while
   [0] Processing  Event run: 323775 lumi: 99 event: 110946721 stream: 0
   [1] Running path 'FEVTDEBUGHLToutput_step'
   [2] Prefetching for module PoolOutputModule/'FEVTDEBUGHLToutput'
   [3] Prefetching for module ReducedRecHitCollectionProducer/'reducedEcalRecHitsEB'
   [4] Prefetching for module InterestingDetIdCollectionProducer/'interestingEcalDetIdPFEB'
   [5] Prefetching for module PFECALSuperClusterProducer/'particleFlowSuperClusterECAL'
   [6] Prefetching for module RecoChargedRefCandidatePrimaryVertexSorter/'offlinePrimaryVertices'
   [7] Prefetching for module ChargedRefCandidateProducer/'trackRefsForJetsBeforeSorting'
   [8] Prefetching for module TrackWithVertexRefSelector/'trackWithVertexRefSelectorBeforeSorting'
   [9] Prefetching for module DuplicateListMerger/'generalTracks'
   [10] Prefetching for module TrackProducer/'mergedDuplicateTracks'
   [11] Prefetching for module DuplicateTrackMerger/'duplicateTrackCandidates'
   [12] Prefetching for module TrackCollectionMerger/'preDuplicateMergingGeneralTracks'
   [13] Prefetching for module TrackCollectionMerger/'earlyGeneralTracks'
   [14] Prefetching for module TrackProducer/'highPtTripletStepTracks'
   [15] Prefetching for module MkFitOutputConverter/'highPtTripletStepTrackCandidates'
   [16] Calling method for module MkFitProducer/'highPtTripletStepTrackCandidatesMkFit'
Exception Message:
MkFitHitWrapper has pixel cluster ID 0:0 but pixel cluster mask has 4:499
----- End Fatal Exception -------------------------------------------------

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/10024.3_TTbar_13+2017_trackingOnlyRun2+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+RecoFakeHLT+HARVESTFakeHLT
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/10024.4_TTbar_13+2017_trackingLowPU+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+RecoFakeHLT+HARVESTFakeHLT
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/148.0_HydjetQ_MinBias_XeXe_5442GeV_2017+HydjetQ_MinBias_XeXe_5442GeV_2017+DIGIHI2017+RECOHI2017+HARVESTHI2017
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/149.0_QCD_Pt_80_120_13_PPREF+QCD_Pt_80_120_13_PPREF+DIGIPPREF2017+RECOPPREF2017+HARVESTPPREF2017

Summary:

  • You potentially added 8683 lines to the logs
  • Reco comparison results: 21892 differences found in the comparisons
  • DQMHistoTests: Total files compared: 40
  • DQMHistoTests: Total histograms compared: 3211080
  • DQMHistoTests: Total failures: 56085
  • DQMHistoTests: Total nulls: 7
  • DQMHistoTests: Total successes: 3154966
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.02 KiB( 39 files compared)
  • DQMHistoSizes: changed ( 136.874 ): -0.020 KiB JetMET/SUSYDQM
  • Checked 169 log files, 37 edm output root files, 40 DQM output files
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 1, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ddcd2/19303/summary.html
COMMIT: 7b270ed
CMSSW: CMSSW_12_1_X_2021-09-30-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35457/19303/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/10024.3_TTbar_13+2017_trackingOnlyRun2+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+RecoFakeHLT+HARVESTFakeHLT
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/10024.4_TTbar_13+2017_trackingLowPU+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+RecoFakeHLT+HARVESTFakeHLT
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/140.0_HydjetQ_B12_5020GeV_2011+HydjetQ_B12_5020GeV_2011+DIGIHI2011+RECOHI2011+HARVESTHI2011
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/140.52_RunHI2010+RunHI2010+RECOHID10+RECOHIR10D11+HARVESTDHI
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/140.54_RunPA2013+RunPA2013+RECO_PPbData+HARVEST_PPbData
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/145.0_HydjetQ_B12_5020GeV_2015+HydjetQ_B12_5020GeV_2015+DIGIHI2015+RECOHI2015+HARVESTHI2015
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/148.0_HydjetQ_MinBias_XeXe_5442GeV_2017+HydjetQ_MinBias_XeXe_5442GeV_2017+DIGIHI2017+RECOHI2017+HARVESTHI2017
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ddcd2/149.0_QCD_Pt_80_120_13_PPREF+QCD_Pt_80_120_13_PPREF+DIGIPPREF2017+RECOPPREF2017+HARVESTPPREF2017

Summary:

  • You potentially added 8702 lines to the logs
  • Reco comparison results: 22836 differences found in the comparisons
  • DQMHistoTests: Total files compared: 40
  • DQMHistoTests: Total histograms compared: 3211080
  • DQMHistoTests: Total failures: 58632
  • DQMHistoTests: Total nulls: 69
  • DQMHistoTests: Total successes: 3152357
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 65.335 KiB( 39 files compared)
  • DQMHistoSizes: changed ( 136.874 ): -0.020 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 140.53 ): 63.680 KiB Hcal/DigiRunHarvesting
  • DQMHistoSizes: changed ( 140.53 ): 1.676 KiB RPC/DCSInfo
  • Checked 169 log files, 37 edm output root files, 40 DQM output files
  • TriggerResults: no differences found

@jpata
Copy link
Contributor

jpata commented Oct 4, 2021

I have no comments on the code. I want to check the outcome of the discussion today at

before signing.

@jpata
Copy link
Contributor

jpata commented Oct 4, 2021

@emilbols any issue with going forward with this PR following the BTV meeting today?

@emilbols
Copy link
Contributor

emilbols commented Oct 4, 2021

@jpata The btv conveners would like to do some additional checks, before approving since a small difference in tagging performance is observed, see [1].

[1]
https://cms-btv-pog-validation.web.cern.ch/cms-btv-pog-validation/validation/packages/RecoB/CMSSW_12_1_0_pre2/TTbar_1210pre2_tkmkfit_2021_14TeV_vs_1210pre2_2021_14TeV_FullSim_PU/deepCSV_BvsAll_performance_vs_B_GLOBAL_all.png

@jpata
Copy link
Contributor

jpata commented Oct 4, 2021

The btv conveners would like to do some additional checks, before approving since a small difference in tagging performance is observed

Could you please clarify if this implies that the sign-off in 12_1_0_pre2_2021_TkmkFitRecoOnly was redacted?

cc @cms-sw/pdmv-l2

@IHateLinus
Copy link

IHateLinus commented Oct 4, 2021

The btv conveners would like to do some additional checks, before approving since a small difference in tagging performance is observed

Could you please clarify if this implies that the sign-off in 12_1_0_pre2_2021_TkmkFitRecoOnly was redacted?

cc @cms-sw/pdmv-l2

There has been quite a few improvements on (the already rather good pre2) and no real show-stoppers found so far. There are legitimate requests for larger statistics tests and those should be made. In discussions last week w @arizzi and @rappoccio , it was agreed that the next phase of validations and larger samples will be done after and using pre4.

@emilbols
Copy link
Contributor

emilbols commented Oct 4, 2021

Could you please clarify if this implies that the sign-off in 12_1_0_pre2_2021_TkmkFitRecoOnly was redacted?

Yes, it was redacted

@jpata
Copy link
Contributor

jpata commented Oct 4, 2021

Thanks for the clarifications. So this looks like a PPD / PdmV question.

@jpata
Copy link
Contributor

jpata commented Oct 4, 2021

+reconstruction

  • mkFit is enabled in certain tracking steps by default, as discussed in the presentation linked in the description
  • since the default is changed, there are expected reco changes in many quantities downstream from tracks
  • this sig is for the code changes from the technical/reco point of view. The decision if and in which release to merge is not for reco, and perhaps can be brought up at the ORP meeting.

@kskovpen
Copy link
Contributor

kskovpen commented Oct 5, 2021

Looks good to PdmV

@perrotta
Copy link
Contributor

perrotta commented Oct 5, 2021

+1

  • Mkfit is enabled in pre4 for the first four tracking iterations, as decided right now at the ORP: a protection is put in place for HI workflows
  • A modifier is defined here to allow switching back to the Run3_noMkFit case

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 5, 2021

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

@cmsbuild cmsbuild merged commit 668228d into cms-sw:master Oct 5, 2021
@perrotta
Copy link
Contributor

perrotta commented Oct 5, 2021

+operations

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

10 participants