search

cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.09k stars 4.32k forks source link

Migrate ED modules to use esConsumes #31061

Closed Dr15Jones closed 1 year ago

Dr15Jones commented 4 years ago

All ED modules need to be migrated to use esConsumes and edm::ESGetToken to provide better threading efficiency.

cmsbuild commented 4 years ago

A new Issue was created by @Dr15Jones Chris Jones.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

Dr15Jones commented 4 years ago

assign l1, reconstruction, simulation, dqm, hlt

cmsbuild commented 4 years ago

New categories assigned: dqm,simulation,hlt,reconstruction,l1

@mdhildreth,@jfernan2,@slava77,@benkrikler,@andrius-k,@fioriNTU,@rekovic,@perrotta,@jpata,@kmaeshima,@Martin-Grunewald,@civanch,@fwyzard you have been requested to review this Pull request/Issue and eventually sign? Thanks

Dr15Jones commented 4 years ago

The following modules were looked at but deemed to difficult to change for a non domain expert

Dr15Jones commented 4 years ago

Relevant information about using esConsumes and edm::ESGetToken can be found at:

https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideHowToGetDataFromES#In_ED_module https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideHowToGetDataFromES#Getting_data_from_EventSetup_wit

Dr15Jones commented 4 years ago

The following modules are known to be used and have not been inspected for esConsumes needs

FixedGridRhoProducer
FixedGridRhoProducerFastjet
GEDGsfElectronCoreProducer
GEDGsfElectronFinalizer
GEDGsfElectronValueMapProducer
GEDPhotonCoreProducer
GEDPhotonProducer
GEMRawToDigiModule
GEMRecHitProducer
GEMSegmentProducer
GamIsoDetIdCollectionProducer
GctRawToDigi
GenJetConstituentSelector
GenJetFlavourInfoPreserver
GenJetMatcher
GenParticlePruner
GlobalHaloDataProducer
GlobalMuonProducer
GlobalTrackQualityProducer
GlobalTrackingRegionEDProducer
GlobalTrackingRegionFromBeamSpotEDProducer
GlobalTrackingRegionWithVerticesEDProducer
GoodSeedProducer
GsfElectronCoreEcalDrivenProducer
GsfElectronProducer
GsfTrackProducer
HBHEIsolatedNoiseReflagger
HBHENoiseFilterResultProducer
HBHEPhase1Reconstructor
HBHEPlan1Combiner
HFEMClusterProducer
HFNoseRawToDigiFake
HFPreReconstructor
HFRecoEcalCandidateProducer
HGCalElectronFilter
HGCalElectronIDValueMapProducer
HGCalLayerClusterProducer
HGCalMultiClusterProducer
HGCalPhotonIDValueMapProducer
HGCalRawToDigiFake
HGCalRecHitMapProducer
HGCalRecHitProducer
HGCalTrackCollectionProducer
HGCalUncalibRecHitProducer
HIMuonTrackingRegionEDProducer
HIPixelTrackFilterProducer
HIProtoTrackFilterProducer
HITrackingRegionForPrimaryVtxEDProducer
HadronAndPartonSelector
HcalHaloDataProducer
HcalHitReconstructor
HcalHitSelection
HcalNoiseInfoProducer
HcalRawToDigi
HcalSimpleReconstructor
HiFJGridEmptyAreaCalculator
HiFJRhoProducer
HitPairEDProducer
HybridClusterProducer
InclusiveCandidateVertexFinder
InclusiveVertexFinder
InterestingDetIdCollectionProducer
InterestingEcalDetIdProducer
InterestingTrackEcalDetIdProducer
IslandClusterProducer
JetChargeProducer
JetCoreClusterSplitter
JetCorrFactorsProducer
JetExtender
JetFlavourClustering
JetFlavourIdentifier
JetIDProducer
JetPartonMatcher
JetPlusTrackProducer
JetSubstructurePacker
JetTagProducer
JetTracksAssociatorAtCaloFace
JetTracksAssociatorAtVertex
JetTracksAssociatorExplicit
L1ECALPrefiringWeightProducer
L1ExtraParticlesProd
L1FastjetCorrectorProducer
L1GlobalTriggerEvmRawToDigi
L1GlobalTriggerRawToDigi
L1GlobalTriggerRecordProducer
L1GtTriggerMenuLiteProducer
L1JPTOffsetCorrectorProducer
L1OffsetCorrectorProducer
L1TCaloUpgradeToGCTConverter
L1TPhysicalEtAdder
L1TRawToDigi
L1TTwinMuxRawToDigi
L6SLBCorrectorProducer
LXXXCorrectorProducer
LogErrorHarvester
LowPtGSFToPackedCandidateLinker
LowPtGSFToTrackLinker
LowPtGsfElectronCoreProducer
LowPtGsfElectronIDProducer
LowPtGsfElectronSCProducer
LowPtGsfElectronSeedProducer
LowPtGsfElectronSeedValueMapsProducer
LumiProducer
MCMatcher
ME0RecHitProducer
ME0SegmentProducer
MEtoEDMConverter
MTDClusterProducer
MTDRecHitProducer
MTDTrackQualityMVAProducer
MTDTrackingRecHitProducer
MTDUncalibratedRecHitProducer
MeasurementTrackerEventProducer
MkFitInputConverter
MkFitProducer
ModifiedGsfElectronProducer
MuIsoDepositCopyProducer
MuIsoDepositProducer
MultShiftMETcorrInputProducer
Multi5x5ClusterProducer
Multi5x5SuperClusterProducer
MultiClustersFromTrackstersProducer
MultiHitFromChi2EDProducer
MultiTrackSelector
MuonIdProducer
MuonMET
MuonMETValueMapProducer
MuonMETcorrInputProducer
MuonProducer
MuonReSeeder
MuonSeedGenerator
MuonSeedMerger
MuonSeedProducer
MuonSelectionTypeValueMapProducer
MuonShowerInformationProducer
MuonSimClassifier
MuonTimingProducer
MuonToTrackingParticleAssociatorEDProducer
NjettinessAdder
OmtfUnpacker
OniaPhotonConversionProducer
OnlineMetaDataRawToDigi
OutsideInMuonSeeder
PATElectronProducer
PATElectronSlimmer
PATGenJetSlimmer
PATIsolatedTrackProducer
PATJetCleaner
PATJetCleanerForType1MET
PATJetProducer
PATJetSlimmer
PATJetUpdater
PATLostTracks
PATMETProducer
PATMETSlimmer
PATMuonProducer
PATMuonSlimmer
PATPFJetMETcorrInputProducer
PATPackedCandidateProducer
PATPackedGenParticleProducer
PATPhotonProducer
PATPhotonSlimmer
PATSecondaryVertexSlimmer
PATTauIDEmbedder
PATTauProducer
PATTauSlimmer
PATTriggerObjectStandAloneSlimmer
PATTriggerProducer
PATVertexSlimmer
PFBadHcalPseudoClusterProducer
PFBlockProducer
PFCandIsolatorFromDeposits
PFCandMETcorrInputProducer
PFCand_AssoMap
PFCandidateFromFwdPtrProducer
PFCandidateFwdPtrProducer
PFCandidatePrimaryVertexSorter
PFCandidateProductFromFwdPtrProducer
PFClusterProducer
PFClusterTimeAssigner
PFConversionProducer
PFDisplacedTrackerVertexProducer
PFDisplacedVertexCandidateProducer
PFDisplacedVertexProducer
PFECALSuperClusterProducer
PFEGammaProducer
PFEGammaToCandidate
PFElecTkProducer
PFElectronTranslator
PFJetConstituentSelector
PFJetFwdPtrProducer
PFJetMETcorrInputProducer
PFLinker
PFMETProducer
PFMultiDepthClusterProducer
PFNuclearProducer
PFPhotonTranslator
PFPileUp
PFProducer
PFRecHitProducer
PFRecoTauChargedHadronProducer
PFRecoTauDiscriminationAgainstElectron
PFRecoTauDiscriminationAgainstElectronDeadECAL
PFRecoTauDiscriminationAgainstElectronMVA6
PFRecoTauDiscriminationAgainstMuon
PFRecoTauDiscriminationAgainstMuon2Container
PFRecoTauDiscriminationAgainstMuonMVA
PFRecoTauDiscriminationByHPSSelection
PFRecoTauDiscriminationByIsolationContainer
PFRecoTauDiscriminationByLeadingObjectPtCut
PFRecoTauDiscriminationByMVAIsolationRun2
PFRecoTauTagInfoProducer
PFSimParticleProducer
PFTICLProducer
PFTauFwdPtrProducer
PFTauPrimaryVertexProducer
PFTauSecondaryVertexProducer
PFTauTransverseImpactParameters
PFTrackProducer
PFV0Producer
ParticleBasedIsoProducer
ParticleFlowForChargedMETProducer
ParticleTowerProducer
PartonSelector
Phase2TrackerClusterizer
PhotonConversionTrajectorySeedProducerFromQuadruplets
PhotonConversionTrajectorySeedProducerFromSingleLeg
PhotonCoreProducer
PhotonEcalPFClusterIsolationProducer
PhotonHcalPFClusterIsolationProducer
PhotonIDProducer
PhotonIDValueMapProducer
PhotonMVAValueMapProducer
PhotonProducer
PileupJetIdProducer
PileupSummaryInfoSlimmer
PixelClusterTagInfoProducer
PixelFitterByConformalMappingAndLineProducer
PixelFitterByHelixProjectionsProducer
PixelInactiveAreaTrackingRegionsSeedingLayersProducer
PixelTrackFilterByKinematicsProducer
PixelTrackProducer
PixelTripletHLTEDProducer
PixelTripletLargeTipEDProducer
PreshowerClusterShapeProducer
PreshowerPhiClusterProducer
PrimaryVertexProducer
PuppiProducer
QGTagger
QuickTrackAssociatorByHitsProducer
RPCAMCRawToDigi
RPCRecHitProducer
RPCTwinMuxRawToDigi
RPCUnpackingModule
RandomEngineStateProducer
RecoChargedRefCandidatePrimaryVertexSorter
RecoJetDeltaRValueMapProducer
RecoJetToPatJetDeltaRValueMapProducer
RecoTauCleaner
RecoTauDiscriminantCutMultiplexer
RecoTauJetRegionProducer
RecoTauPiZeroProducer
RecoTauPiZeroUnembedder
RecoTauProducer
ReducedEGProducer
ReducedESRecHitCollectionProducer
ReducedRecHitCollectionProducer
SETMuonSeedProducer
ScalersRawToDigi
SecondaryVertexProducer
SeedClusterRemover
SeedClusterRemoverPhase2
SeedCombiner
SeedCreatorFromRegionConsecutiveHitsEDProducer
SeedCreatorFromRegionConsecutiveHitsTripletOnlyEDProducer
SeedGeneratorFromRegionHitsEDProducer
SeedingLayersEDProducer
ShiftedPATJetProducer
ShiftedParticleMETcorrInputProducer
ShiftedParticleProducer
SiPixelClusterProducer
SiPixelClusterShapeCacheProducer
SiPixelRawToDigi
SiPixelRecHitConverter
SiStripClusterizer
SiStripRawToDigiModule
SiStripRecHitConverter
SiStripZeroSuppression
SimPFProducer
SmearedPATJetProducer
SoftKillerProducer
SoftPFElectronTagInfoProducer
SoftPFMuonTagInfoProducer
StandAloneMuonProducer
SuperClusterProducer
TICLCandidateFromTrackstersProducer
TICLLayerTileProducer
TICLSeedingRegionProducer
TOFPIDProducer
TPPFCandidatesOnPFCandidates
TPPFJetsOnPFCandidates
TPPFTausOnPFJetsDeltaR
TauGenJetProducer
TauRegionalPixelSeedTrackingRegionEDProducer
TcdsRawToDigi
TevMuonProducer
TotemRPClusterProducer
TotemRPLocalTrackFitter
TotemRPRecHitProducer
TotemRPUVPatternFinder
TotemTimingLocalTrackFitter
TotemTimingRecHitProducer
TotemTriggerRawToDigi
TotemVFATRawToDigi
TrackCandidateProducer
TrackClusterRemover
TrackCollectionMerger
TrackCutClassifier
TrackExtenderWithMTD
TrackExtrapolator
TrackIPProducer
TrackListMerger
TrackLwtnnClassifier
TrackMVAClassifierDetached
TrackMVAClassifierPrompt
TrackProducer
TrackProducerWithSCAssociation
TrackTimeValueMapProducer
TrackVertexArbitrator
TrackWithVertexRefSelector
TrackstersMergeProducer
TrackstersProducer
TrajectorySeedFromMuonProducer
Type0PFMETcorrInputProducer
Type2CorrectionProducer
UnifiedSCCollectionProducer
V0Producer
VersionedGsfElectronIdProducer
VersionedPhotonIdProducer
VertexMerger
ZdcHitReconstructor
photonIsolationHIProducer
Dr15Jones commented 4 years ago

The following shows the state of known used modules

https://docs.google.com/spreadsheets/d/1NuE0cgisQGEXxG0gBMg4aNWsbY9ekUmULC5jRw383Ak/edit?ts=5f283f40#gid=0

fwyzard commented 4 years ago

If possible, please do not migrate these modules

SiPixelClusterProducer
SiPixelRecHitConverter
PixelTrackProducer

until the Patatrack branch has been integrated upstream (see e.g. #27983).

makortel commented 4 years ago

I took the static analyzer report (on the non-token calls to EventSetup), and crafted another spreadsheet with a list of packages containing (non-test) EDModules that need to be migrated. I hope we could use it as a synchronization tool to avoid duplicate effort https://docs.google.com/spreadsheets/d/1cUypk2EK4xVcEpFGtaNqXrWZEoFz_E9l4gNN3YoBQH4/edit?usp=sharing

(it already includes @fwyzard's comment https://github.com/cms-sw/cmssw/issues/31061#issuecomment-669269815 )

makortel commented 3 years ago

The IB's provide now a list of EDModules that call the deprecated EventSetupRecord::get(), see e.g. https://cmssdt.cern.ch/SDT/jenkins-artifacts/ib-static-analysis/CMSSW_11_3_X_2021-03-01-2300/slc7_amd64_gcc900/reports/eventsetuprecord-get.txt Note that this list is filtered to show only those EDModules that are declared in any configuration file in the full runTheMatrix.py. There are about 230 such EDModules.

makortel commented 3 years ago

Starting from CMSSW_11_3_X_2021-04-10-1100 EDLoopers can declare Event and EventSetup data products they consume. Therefore the code using EDLooper and helper code being used by EDLooper and e.g. EDProducer (e.g. alignment) can now be migrated. FYI @cms-sw/alca-l2

jan-kaspar commented 3 years ago

From https://docs.google.com/spreadsheets/d/1cUypk2EK4xVcEpFGtaNqXrWZEoFz_E9l4gNN3YoBQH4/edit#gid=0 I picked up RecoPPS/Local for migration, but actually as far as I can tell, all modules are already using the esConsumes/token mechanism. Can you please confirm that I haven't overlooked anything? @fabferro FYI

jan-kaspar commented 3 years ago

From https://docs.google.com/spreadsheets/d/1cUypk2EK4xVcEpFGtaNqXrWZEoFz_E9l4gNN3YoBQH4/edit#gid=0 I picked up Validation/CTPPS for migration, but actually as far as I can tell, all modules are already using the esConsumes/token mechanism. Can you please confirm that I haven't overlooked anything? @fabferro FYI

jfernan2 commented 3 years ago

From https://docs.google.com/spreadsheets/d/1cUypk2EK4xVcEpFGtaNqXrWZEoFz_E9l4gNN3YoBQH4/edit#gid=0 I picked >up Validation/CTPPS for migration, but actually as far as I can tell, all modules are already using the esConsumes/token >mechanism. Can you please confirm that I haven't overlooked anything? @fabferro FYI

I confirm that scram build checker on Validation/CTPSS only yields: /tmp/jfernan/CMSSW_12_0_X_2021-05-25-2300/src/Validation/CTPPS/test/TestDriver.cc:3:1: warning: Non-const variable 'char environ' is static and might be thread-unsafe [threadsafety.GlobalStatic] RUNTEST() ^ /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_12_0_X_2021-05-25-2300/src/FWCore/Utilities/interface/TestHelper.h:30:14: note: expanded from macro 'RUNTEST' extern "C" char environ; \ ^ 1 warning generated.

jan-kaspar commented 3 years ago

Thanks @jfernan2 - I've updated the spread sheet noting that there is nothing to migrate.

makortel commented 3 years ago

Many thanks for the effort so far! It seems that to meet the goal of all runTheMatrix workflows migrated by 12_1_0 we need to speed up a bit. The 2021-07-05-2300 IB shows 155 EDModules to be migrated left in runTheMatrix workflows (link)

I classified these EDModules by L2 categories (not exclusive):

@cms-sw/alca-l2: 2 modules

AlignmentProducerAsAnalyzer
MuonMillepedeTrackRefitter

@cms-sw/analysis-l2: 6 modules

CalibratedElectronProducerRun2T
CalibratedElectronProducerRun2T<pat::Electron>
CalibratedElectronProducerRun2T<reco::GsfElectron>
GenParticlePruner
MuScleFitMuonProducer
PFCand_AssoMap

@cms-sw/fastsim-l2: 7 modules

FastSimProducer
FastTrackDeDxProducer
FastTrackerRecHitMatcher
MuonSimHitProducer
TrackCandidateProducer
TrackingRecHitProducer
TrajectorySeedProducer

@cms-sw/generators-l2: 2 modules

GenParticles2HepMCConverter
PythiaFilterIsolatedTrack

@cms-sw/hlt-l2: 3 modules

L2MuonIsolationProducer
L2MuonProducer
L3MuonCombinedRelativeIsolationProducer

@cms-sw/l1-l2: 19 modules

HFNoseVFEProducer
HGCalBackendLayer1Producer
HGCalBackendLayer2Producer
HGCalConcentratorProducer
HGCalTowerMapProducer
HGCalTowerProducer
HGCalVFEProducer
L1Comparator
L1FPGATrackProducer
L1GtTrigReport
L1RCTProducer
L1TGlobalPrescaler
L1TMuonBarrelKalmanStubProducer
L1TMuonBarrelTrackProducer
L1TMuonEndCapTrackProducer
L1TTwinMuxProducer
RPCTechnicalTrigger
omtf::OmtfPacker
omtf::OmtfUnpacker

@cms-sw/pdmv-l2: 1 modules

LeptonSkimming

@cms-sw/reconstruction-l2: 103 modules

@cms-sw/simulation-l2: 15 modules

CSCDigiProducer
ESZeroSuppressionProducer
EcalSelectiveReadoutProducer
MuonAssociatorEDProducer
MuonToTrackingParticleAssociatorEDProducer
OscarMTProducer
PPSSimTrackProducer
RPDigiProducer
SeedToTrackProducer
TrackAssociatorByChi2Producer
TrackAssociatorByHitsProducer
TrackAssociatorByPositionProducer
TrackTimeValueMapProducer
TrackingParticleNumberOfLayersProducer
edm::BMixingModule

@cms-sw/upgrade-l2: 8 modules

GEMCSCSegmentProducer
HFNoseVFEProducer
HGCalBackendLayer1Producer
HGCalBackendLayer2Producer
HGCalConcentratorProducer
HGCalTowerMapProducer
HGCalTowerProducer
HGCalVFEProducer

To remind, the plan was to start to mark PRs that modify any file accessing EventSetup without ESGetToken as failed in 12_1_X.

makortel commented 3 years ago

assign alca, analysis, fastsim, generators, pdmv, upgrade

cmsbuild commented 3 years ago

New categories assigned: fastsim,pdmv,upgrade,analysis,alca,generators

@francescobrivio,@civanch,@sbein,@tvami,@malbouis,@pohsun,@mkirsano,@jordan-martins,@ssekmen,@SiewYan,@srimanob,@alberto-sanchez,@chayanit,@wajidalikhan,@kpedro88,@tlampen,@mdhildreth,@santocch,@yuanchao,@agrohsje,@lveldere,@GurpreetSinghChahal you have been requested to review this Pull request/Issue and eventually sign? Thanks

agrohsje commented 3 years ago

@mkirsano @SiewYan are you available to follow-up on the remaining gen points? i will be offline for 2 weeks. @qliphy will help with signing our pr's.

mkirsano commented 3 years ago

Yes, I am available, should just see what is to be handled

08/07/21 11:38, agrohsje пишет:

@mkirsano https://github.com/mkirsano @SiewYan https://github.com/SiewYan are you available to follow-up on the remaining gen points? i will be offline for 2 weeks. @qliphy https://github.com/qliphy will help with signing our pr's.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cms-sw/cmssw/issues/31061#issuecomment-876291244, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGOJURPHVOLTCFGBRFRR5DTWVWYXANCNFSM4PVSASAA.

fwyzard commented 3 years ago

@makortel , these three

@cms-sw/hlt-l2: 3 modules

L2MuonIsolationProducer
L2MuonProducer
L3MuonCombinedRelativeIsolationProducer

do not use the EventSetup directly:

makortel commented 3 years ago

@fwyzard

@cms-sw/hlt-l2: 3 modules

L2MuonIsolationProducer
L2MuonProducer
L3MuonCombinedRelativeIsolationProducer

do not use the EventSetup directly:

Thanks for the analysis.

  • L2MuonProducer uses it via RecoMuon/TrackingTools (and it looks like it has already been updated to use esConsumes ?);

There is a possible call (via virtual functions) here https://github.com/cms-sw/cmssw/blob/123c40d963a9cac49f968d6c86a6b247058aa0b5/RecoMuon/StandAloneTrackFinder/src/StandAloneMuonRefitter.cc#L42

  • L2MuonIsolationProducer and L3MuonCombinedRelativeIsolationProducer use it via PhysicsTools/IsolationAlgos.

Right. In any case the code-to-be-migrated is in reconstruction (with a possible impact on these EDModules, I did not check that deeply). I won't tag HLT for these modules in subsequent updates.

srimanob commented 3 years ago

@cms-sw/hgcal-dpg-l2 @cms-sw/hcal-dpg-l2 @cms-sw/gem-dpg-l2 @cms-sw/csc-dpg-l2

Could you please take a look at the modules listed by @makortel in (*) including

GEMCSCSegmentProducer
HFNoseVFEProducer
HGCalBackendLayer1Producer
HGCalBackendLayer2Producer
HGCalConcentratorProducer
HGCalTowerMapProducer
HGCalTowerProducer
HGCalVFEProducer

Thanks.

(*) https://github.com/cms-sw/cmssw/issues/31061#issuecomment-875653146

hyunyong commented 3 years ago

Many thanks for the effort so far! It seems that to meet the goal of all runTheMatrix workflows migrated by 12_1_0 we need to speed up a bit. The 2021-07-05-2300 IB shows 155 EDModules to be migrated left in runTheMatrix workflows (link)

I classified these EDModules by L2 categories (not exclusive):

@cms-sw/alca-l2: 2 modules

AlignmentProducerAsAnalyzer
MuonMillepedeTrackRefitter

@cms-sw/analysis-l2: 6 modules

CalibratedElectronProducerRun2T
CalibratedElectronProducerRun2T<pat::Electron>
CalibratedElectronProducerRun2T<reco::GsfElectron>
GenParticlePruner
MuScleFitMuonProducer
PFCand_AssoMap

@cms-sw/fastsim-l2: 7 modules

FastSimProducer
FastTrackDeDxProducer
FastTrackerRecHitMatcher
MuonSimHitProducer
TrackCandidateProducer
TrackingRecHitProducer
TrajectorySeedProducer

@cms-sw/generators-l2: 2 modules

GenParticles2HepMCConverter
PythiaFilterIsolatedTrack

@cms-sw/hlt-l2: 3 modules

L2MuonIsolationProducer
L2MuonProducer
L3MuonCombinedRelativeIsolationProducer

@cms-sw/l1-l2: 19 modules

HFNoseVFEProducer
HGCalBackendLayer1Producer
HGCalBackendLayer2Producer
HGCalConcentratorProducer
HGCalTowerMapProducer
HGCalTowerProducer
HGCalVFEProducer
L1Comparator
L1FPGATrackProducer
L1GtTrigReport
L1RCTProducer
L1TGlobalPrescaler
L1TMuonBarrelKalmanStubProducer
L1TMuonBarrelTrackProducer
L1TMuonEndCapTrackProducer
L1TTwinMuxProducer
RPCTechnicalTrigger
omtf::OmtfPacker
omtf::OmtfUnpacker

@cms-sw/pdmv-l2: 1 modules

LeptonSkimming

@cms-sw/reconstruction-l2: 103 modules

@cms-sw/simulation-l2: 15 modules

CSCDigiProducer
ESZeroSuppressionProducer
EcalSelectiveReadoutProducer
MuonAssociatorEDProducer
MuonToTrackingParticleAssociatorEDProducer
OscarMTProducer
PPSSimTrackProducer
RPDigiProducer
SeedToTrackProducer
TrackAssociatorByChi2Producer
TrackAssociatorByHitsProducer
TrackAssociatorByPositionProducer
TrackTimeValueMapProducer
TrackingParticleNumberOfLayersProducer
edm::BMixingModule

@cms-sw/upgrade-l2: 8 modules

GEMCSCSegmentProducer
HFNoseVFEProducer
HGCalBackendLayer1Producer
HGCalBackendLayer2Producer
HGCalConcentratorProducer
HGCalTowerMapProducer
HGCalTowerProducer
HGCalVFEProducer

To remind, the plan was to start to mark PRs that modify any file accessing EventSetup without ESGetToken as failed in 12_1_X.

I'm working on the AlignmentProducerAsAnalyzer module.

makortel commented 3 years ago

The script producing the list of EDModules-to-be-migrated-run-in-runTheMatrix for each IB had some flaws that caused e.g. many (or recently all) DQM modules to be missing. The script has been fixed now, and updated list of modules classified according to L2 areas (not exclusive) from CMSSW_12_0_X_2021-07-19-2300 can be found below. The YAML file shows also now (again?) the full function call stacks from the EDModule to the point of EventSetupRecord::get(). (somehow the script listed previously also a few EDModules not actually run in runTheMatrix; those are not listed anymore, but eventually they'd need to be migrated too)

@cms-sw/alca-l2: 1 module

AlignmentProducerAsAnalyzer

@cms-sw/analysis-l2: 2 modules

GenParticlePruner
PFCand_AssoMap

@cms-sw/dqm-l2: 13 modules

BDHadronTrackMonitoringAnalyzer
ECALMultifitAnalyzer_HI
EcalBarrelRecHitsValidation
EcalDQMonitorTask
EcalEndcapRecHitsValidation
EcalRecHitsValidation
EcalSelectiveReadoutValidation
MuonTrackValidator
SingleTopTChannelLeptonDQM
SingleTopTChannelLeptonDQM_miniAOD
TopDiLeptonOfflineDQM
TopSingleLeptonDQM
TopSingleLeptonDQM_miniAOD

@cms-sw/fastsim-l2: 7 modules

FastSimProducer
FastTrackDeDxProducer
FastTrackerRecHitMatcher
MuonSimHitProducer
TrackCandidateProducer
TrackingRecHitProducer
TrajectorySeedProducer

@cms-sw/generators-l2: 1 module

GenParticles2HepMCConverter

HLT, actual EventSetupRecord::get() calls are in reconstruction code

L2MuonIsolationProducer
L2MuonProducer
L3MuonCombinedRelativeIsolationProducer

@cms-sw/l1-l2: 18 modules

HFNoseVFEProducer
HGCalBackendLayer1Producer
HGCalBackendLayer2Producer
HGCalConcentratorProducer
HGCalTowerMapProducer
HGCalTowerProducer
HGCalVFEProducer
L1Comparator
L1FPGATrackProducer
L1GtTrigReport
L1RCTProducer
L1TMuonBarrelKalmanStubProducer
L1TMuonBarrelTrackProducer
L1TMuonEndCapTrackProducer
L1TTwinMuxProducer
RPCTechnicalTrigger
omtf::OmtfPacker
omtf::OmtfUnpacker

@cms-sw/reconstruction-l2: 94 modules

@cms-sw/simulation-l2: 16 modules

CSCDigiProducer
ESZeroSuppressionProducer
EcalSelectiveReadoutProducer
MuonAssociatorEDProducer
MuonToTrackingParticleAssociatorEDProducer
OscarMTProducer
PPSSimTrackProducer
PreMixingModule
RPDigiProducer
SeedToTrackProducer
TrackAssociatorByChi2Producer
TrackAssociatorByHitsProducer
TrackAssociatorByPositionProducer
TrackTimeValueMapProducer
TrackingParticleNumberOfLayersProducer
edm::BMixingModule

@cms-sw/upgrade-l2: 7 modules

HFNoseVFEProducer
HGCalBackendLayer1Producer
HGCalBackendLayer2Producer
HGCalConcentratorProducer
HGCalTowerMapProducer
HGCalTowerProducer
HGCalVFEProducer
srimanob commented 3 years ago

The following modules will be taken care by HGCAL trigger group,

HFNoseVFEProducer
HGCalBackendLayer1Producer
HGCalBackendLayer2Producer
HGCalConcentratorProducer
HGCalTowerMapProducer
HGCalTowerProducer
HGCalVFEProducer

@jbsauvan

jfernan2 commented 3 years ago

@makortel Regarding:

BDHadronTrackMonitoringAnalyzer MuonTrackValidator SingleTopTChannelLeptonDQM SingleTopTChannelLeptonDQM_miniAOD TopDiLeptonOfflineDQM TopSingleLeptonDQM TopSingleLeptonDQM_miniAOD

Those modules were migrated in what esConsumes concerns in PRs : https://github.com/cms-sw/cmssw/pull/34296/ https://github.com/cms-sw/cmssw/pull/34365/ https://github.com/cms-sw/cmssw/pull/34266/

in which the tests did not give any other mention to esConsumes not migrated yet. Indeed, a 'scram build checker' over the current version of the packages does not mention any module to migrate either.

How can we check where the ofending lines are? Thank you

tvami commented 3 years ago

@cms-sw/alca-l2: 1 module

AlignmentProducerAsAnalyzer

I think this is taken care of in the ongoing PR https://github.com/cms-sw/cmssw/pull/34542

makortel commented 3 years ago

@jfernan2

Those modules were migrated in what esConsumes concerns in PRs :

34296

34365

34266

in which the tests did not give any other mention to esConsumes not migrated yet.

The PR tests check only the files touched in a PR.

Indeed, a 'scram build checker' over the current version of the packages does not mention any module to migrate either.

In this case the code-to-be-migrated is in helper classes that are (likely) in separate package.

How can we check where the ofending lines are?

From the latest report

BDHadronTrackMonitoringAnalyzer

MuonTrackValidator

SingleTopTChannelLeptonDQM

SingleTopTChannelLeptonDQM_miniAOD

TopDiLeptonOfflineDQM

TopSingleLeptonDQM

TopSingleLeptonDQM_miniAOD

Addressing the https://github.com/cms-sw/cmssw/blob/0805f82c66e2460cf5eaab85be533642a3149cbb/DQM/Physics/interface/TopDQMHelpers.h#L237-L238 would fix many of these (by quick look the JetCorrector::getJetCorrector() does not look very useful anymore with ESGetToken).

jfernan2 commented 3 years ago

would fix many of these (by quick look the JetCorrector::getJetCorrector() does not look very useful anymore with ESGetToken).

I am not sure to understand exactly this: getJetCorrector seems to be the only method to access JetCorrections https://github.com/cms-sw/cmssw/blob/6d2f66057131baacc2fcbdd203588c41c885b42c/JetMETCorrections/Objects/src/JetCorrector.cc#L49 and it is not using esConsumes since it has not been migrated yet (row 194 of https://docs.google.com/spreadsheets/d/1cUypk2EK4xVcEpFGtaNqXrWZEoFz_E9l4gNN3YoBQH4/edit#gid=0 )

makortel commented 3 years ago

@jfernan2 The only thing JetCorrector::getJetCorrector() does is to get the JetCorrector object with a given label from EventSetup https://github.com/cms-sw/cmssw/blob/6d2f66057131baacc2fcbdd203588c41c885b42c/JetMETCorrections/Objects/src/JetCorrector.cc#L48-L53

Thefore the calling code in https://github.com/cms-sw/cmssw/blob/6d2f66057131baacc2fcbdd203588c41c885b42c/DQM/Physics/interface/TopDQMHelpers.h#L474 could be replaced simply with

corrector = &setup.getData(esToken);

(and eventually remove the JetCorrector::getJetCorrector()).

jfernan2 commented 3 years ago

Thanks for the tips @makortel Unfortunately, I am sorry but, despite I have been struggling with this most of the day, I am not able to solve the problems, due to my lack of understanding of the situation. Which command produces the Report? How can I check my changes are in the right direction? Is there any scram b command giving the complain for this kind of issues? My current (stuck) status is the following:

Any help is really appreciated. Thanks in advance

makortel commented 3 years ago

@jfernan2 No worries, let's figure these out.

Which command produces the Report? How can I check my changes are in the right direction?

The report is done by post-processing the static analyzer output, but I don't know if there is an easy way to run those yourself (probably is and I'm just ignorant). @gartung @smuzaffar ?

Any change from EventSetupRecord::get() to EventSetup::getData()/EventSetup::getHandle() is in the right direction.

  • For BDHadronTrackMonitoringAnalyzer, TrackClassifier seems to not be calling any ES, so I don't get from the report the exact point causing the conflict

TrackClassifier::newEvent() uses EventSetup here https://github.com/cms-sw/cmssw/blob/a8808605e3a4a125035acc759bfbea22c52d09da/SimTracker/TrackHistory/src/TrackClassifier.cc#L54-L58 https://github.com/cms-sw/cmssw/blob/a8808605e3a4a125035acc759bfbea22c52d09da/SimTracker/TrackHistory/src/TrackClassifier.cc#L63-L64 https://github.com/cms-sw/cmssw/blob/a8808605e3a4a125035acc759bfbea22c52d09da/SimTracker/TrackHistory/src/TrackClassifier.cc#L69-L72

  • For MuonTrackValidator, I identified a possible conflict and commented it, it compiles, but I don't know how to check if it solves the issue. ae14ab3

The commit comments out a consumes() call to Event product. The base class does the same consumes() call https://github.com/cms-sw/cmssw/blob/ae14ab3c8cbb5ccaab0295d1264e698b4511dbe0/Validation/RecoMuon/plugins/MuonTrackValidatorBase.h#L44 and the token is used here https://github.com/cms-sw/cmssw/blob/ae14ab3c8cbb5ccaab0295d1264e698b4511dbe0/Validation/RecoMuon/plugins/MuonTrackValidator.cc#L425 (this structure is a little bit confusing and more complex than necessary)

The calls that need to be migrated are here https://github.com/cms-sw/cmssw/blob/ae14ab3c8cbb5ccaab0295d1264e698b4511dbe0/CommonTools/RecoAlgos/interface/CosmicTrackingParticleSelector.h#L106-L112

  • For JetCorrector, I tried to mimic the solution used in other parts of CMSSW but it does not compile, I am not sure if due to the template or because the particularities of JetCorrector. 30fdc43

Your commit declares consumes for Event product, and tries to read it from the Event. The changes should be along

edm::ESGetToken<JetCorrector, JetCorrectionsRecord> jetCorrector_;
...
jetCorrector_ = iC.esConsumes(edm::ESInputTag("", cfg.getParameter<std::string>("jetCorrector")));
...
corrector = &setup.getData(jetCorrector_);
jfernan2 commented 3 years ago

Thank you very much @makortel I have created https://github.com/cms-sw/cmssw/pull/34604 following your tips Only DQM/Ecal packages are missing form this esConsumes migration, and they are being taken care by ECAL dpg

abhih1 commented 3 years ago

The Ecal DQM, DQMOffline and Validation packages are migrated in here respectively: https://github.com/cms-sw/cmssw/pull/34634, https://github.com/cms-sw/cmssw/pull/34636 and https://github.com/cms-sw/cmssw/pull/34637

makortel commented 3 years ago

@cms-sw/alca-l2 There are still some helper function calls from AlignmentProducerAsAnalyzer in the latest report

Would you be able to migrate these as well?

tvami commented 3 years ago

@cms-sw/alca-l2 There are still some helper function calls from AlignmentProducerAsAnalyzer in the latest report

  • BzeroReferenceTrajectoryFactory::trajectories()
  • DualBzeroTrajectoryFactory::trajectories()
  • DualTrajectoryFactory::trajectories()
  • ReferenceTrajectoryFactory::trajectories()
  • TwoBodyDecayTrajectoryFactory::trajectories()
  • MuonResidualsFromTrack constructor
  • SiPixelLorentzAngleCalibration::beginRun()
  • SiStripLorentzAngleCalibration::beginRun()

Would you be able to migrate these as well?

Hi @mmusich @hyunyong can you please have a look at these?

mmusich commented 3 years ago

Hi @mmusich can you please have a look at these?

I am afraid that the answer is no - at least in the near term future, but @bbilin has pledged to work on this, so I invite to redirect your queries to him directly.

bbilin commented 3 years ago

I will have a look in the upcoming days and migrate those. Thanks @mmusich for pinging.

tvami commented 3 years ago

I will have a look in the upcoming days and migrate those. Thanks @mmusich for pinging.

Hi @bbilin have you had the chance to look into this?

bbilin commented 3 years ago

Hi @tvami I finally managed to start today. I hope to progress before the end of the week. Thanks for following up.

makortel commented 3 years ago

The following modules will be taken care by HGCAL trigger group,

HFNoseVFEProducer
HGCalBackendLayer1Producer
HGCalBackendLayer2Producer
HGCalConcentratorProducer
HGCalTowerMapProducer
HGCalTowerProducer
HGCalVFEProducer

@jbsauvan

Hi, any news about these?

makortel commented 3 years ago

I finally managed to start today. I hope to progress before the end of the week. Thanks for following up.

Hi @bbilin, would you happen to have any news on AlignmentProducerAsAnalyzer yet?

tvami commented 3 years ago

I finally managed to start today. I hope to progress before the end of the week. Thanks for following up.

Hi @bbilin, would you happen to have any news on AlignmentProducerAsAnalyzer yet?

Hi @makortel my understanding of the situation is that the changes in AlignmentProducerAsAnalyzer lead very deep in the code, so the Tracking POG (@mmusich) would prefer to have a more extensive validation done before @bbilin submits the PR.

mmusich commented 3 years ago

@tvami the code to be touched is not maintained by the POG, but rather from the Tracker Alignment group. As I have tackled similar migrations in the past I suggested @bbilin certain tests to make sure there's no functionality disruption, a part from having code that is superficially compiling. I have no idea of the status of those tests.

jfernan2 commented 3 years ago

+1 All DQM modules have been migrated as far as I can tell

jbsauvan commented 3 years ago

The following modules will be taken care by HGCAL trigger group,

HFNoseVFEProducer
HGCalBackendLayer1Producer
HGCalBackendLayer2Producer
HGCalConcentratorProducer
HGCalTowerMapProducer
HGCalTowerProducer
HGCalVFEProducer

@jbsauvan

Hi, any news about these?

The migration for HGCAL trigger modules is done in #35459

makortel commented 3 years ago

Just to note that static analyzer still reports hits in AlCa code, but they are not in the known set of EDModules run in IB tests.

tvami commented 3 years ago

Hi @makortel can you please point me to this list? Thanks!

makortel commented 3 years ago

In the IB dashboard click "Static Analyzer" (leads to e.g. https://cmssdt.cern.ch/SDT/jenkins-artifacts/ib-static-analysis/CMSSW_12_2_X_2021-11-03-2300/slc7_amd64_gcc900/llvm-analysis/index.html), and there uncheck "Display?" for "All bugs", and then check "Display?" for "EventSetupRecord::get function called".

Alternatively see the compilation warnings for CMSDEPRECATED_X IB, e.g. https://cmssdt.cern.ch/SDT/cgi-bin/showBuildLogs.py/slc7_amd64_gcc900/www/wed/12.2.CMSDEPRECATED-wed-23/CMSSW_12_2_CMSDEPRECATED_X_2021-11-03-2300 (these contain also warnings from legacy EDModules)