-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Replace SeedingLayersESProducer with an EDProducer #2286
Conversation
…s unused This simplifies further development of SeedingLayerSetsBuilder.
…nterface The old interface needs to be kept, because some classes use it with ctfseeding:SeedingLayer's constructed without SeedingLayersESProducer. I used templates in CosmicTrackingRegion and RectangularEtaPhiTrackingRegion to avoid copy-pasting.
…, MultiHitGeneratorFromChi2
In QuadrupletSeedMerger SeedingLayerSets are used only to iterate over layer-quadruplets. Therefore SeedingLayersEDProducer would mean unnecessary work by creating the TTRHs. Therefore I decided to just deliver the SeedingLayerSetsBuilder PSet to QuadrupletSeedMerger, and construct SeedingLayerSetsBuilder in the constructor.
Will need to deliver edm::ConsumesCollector to OrderedHitsGenerator in SeedFilter (to be added in a later commit). As a knock-on effect, move SeedFilter construction from beginRun() to constructor in ElectronSeedProducer. Also, use std::unique_ptr for SeedFilter.
Will be needed to deliver edm::ConsumesCollector to - CombinedHitPairGenerator - CombinedMultiHitGenerator - CombinedHitTripletGenerator - CombinedHitPairGeneratorForPhotonConversion - CombinedHitQuadrupletGeneratorForPhotonConversion Following concrete classes inherit from OrderedHitsGenerator - BeamHaloPairGenerator - GenericPairGenerator - GenericTripletGenerator - CombinedHitPairGenerator - CombinedMultiHitGenerator - CombinedHitTripletGenerator - CombinedHitPairGeneratorForPhotonConversion - CombinedHitQuadrupletGeneratorForPhotonConversion Knock-on effects: Pass ConsumesCollector to OrderedHitsFactory - TSGSmart + move construction of OrderedHitsGenerator to constructor - TSGFromL1Muon (from beginRun) - PixelTrackReconstruction (from beginRun) - HitTripletProducer (from init) - CtfSpecialSeedGenerator (from beginRun) - PhotonConversionTrajectorySeedProducerFromSingleLegAlgo (from init) - ConversionSeedGenerators/src/PhotonConversionTrajectorySeedProducerFromQuadrupletsAlgo (from init) + move construction of SeedGeneratorFromRegionHits to constructor - TSGFromOrderedHits - EgammaHLTRegionalPixelSeedGeneratorProducers + move construction of SeedComparitor and SeedCreator to constructor - SeedGeneratorFromRegionHitsEDProducer
Needed for edm::ConsumesCollector (in later commit).
Hi David, On 05/02/14 15:59, dmoon wrote:
Apologies from my part for the spam. I guess you got included because I think you can remove yourself by clicking "Unsubscribe" on the right Cheers, |
@dmoon, I think I added you by accident, sorry about that. Should be fixed now. |
… GlobalTrackingRegionProducerFromBeamSpot
Thanks Matti, I’ll try that. Thanks, David Moon This e-mail and any attachments thereto are the property of OnAsset Intelligence, Inc and may contain confidential material. Its use is solely for the intended recipient(s). Any review, use, distribution, disclosure or copying of this e-mail or attachments by others is strictly prohibited. If you received this e-mail in error, please contact the sender and permanently delete the original message. From: Matti Kortelainen [mailto:notifications@github.com] Hi David, On 05/02/14 15:59, dmoon wrote:
Apologies from my part for the spam. I guess you got included because I think you can remove yourself by clicking "Unsubscribe" on the right Cheers, — |
Trying to generate HIon workflows with cmsDriver, I get the following problem: Creating RECO+DQM HIon_DATA The line 34 creating the problem contains the py code: SEEDING LAYERShiRegitPixelPairStepSeedLayers = RecoTracker.IterativeTracking.PixelPairStep_cff.pixelPairStepSeedLayers.clone( Since with the move from ESP to EDP the parameter ComponentName no longer exists |
Hi Martin, Whoops, apparently I missed these ones while Cheers, |
Here are first the fixes to PhotonConversionTrajectorySeedProducerFromSingleLegAlgo and
and none failed because of this migration (some upgrade configurations did fail for other reasons, and hence might hide failures relevant to this PR). Again, no need to re-run the tests yet as the ones containing HLT will still fail. |
Pull request #2286 was updated. @perrotta, @cmsbuild, @civanch, @diguida, @lveldere, @fwyzard, @ianna, @mdhildreth, @Martin-Grunewald, @anton-a, @Dr15Jones, @rcastello, @giamman, @slava77, @Degano, @ktf, @thspeer, @nclopezo can you please check and sign again. |
Hi, GRun and PIon seem to work. FastSim and HIon do not work. In case of HIon the error is: ----- Begin Fatal Exception 07-Feb-2014 15:34:19 CET----------------------- Additional Info: This is weird: the original HIon config (HLTrigger/Configuration/python/HLT_HIon_cff.py) Now with the new SeedingLayerEDProducer, here named hltPixelLayerTriplets (having dropped the "ESP" from the label), and same configuration as the old hltESPPixelLayerTriplets, it know What is going on here? |
-1 runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT.log ----- Begin Fatal Exception 07-Feb-2014 16:21:15 CET----------------------- An exception of category 'Configuration' occurred while [0] Constructing the EventProcessor [1] Validating configuration of module: class=EgammaHLTRegionalPixelSeedGeneratorProducers label='hltL1SeededEgammaRegionalPixelSeedGenerator' Exception Message: Parameter "SeedingLayers" should be defined as a tracked InputTag. The type in the configuration is incorrect. ----- End Fatal Exception ------------------------------------------------- 5.1 step1 runTheMatrix-results/5.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log 401.0 step1 runTheMatrix-results/401.0_TTbarNewMix+TTbarFSPU2+HARVESTFS/step1_TTbarNewMix+TTbarFSPU2+HARVESTFS.log 8.0 step2 runTheMatrix-results/8.0_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS/step2_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS.log ----- Begin Fatal Exception 07-Feb-2014 16:24:21 CET----------------------- An exception of category 'Configuration' occurred while [0] Constructing the EventProcessor [1] Validating configuration of module: class=EgammaHLTRegionalPixelSeedGeneratorProducers label='hltL1SeededEgammaRegionalPixelSeedGenerator' Exception Message: Parameter "SeedingLayers" should be defined as a tracked InputTag. The type in the configuration is incorrect. ----- End Fatal Exception ------------------------------------------------- 1306.0 step2 runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step2_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log ----- Begin Fatal Exception 07-Feb-2014 16:25:06 CET----------------------- An exception of category 'Configuration' occurred while [0] Constructing the EventProcessor [1] Validating configuration of module: class=EgammaHLTRegionalPixelSeedGeneratorProducers label='hltL1SeededEgammaRegionalPixelSeedGenerator' Exception Message: Parameter "SeedingLayers" should be defined as a tracked InputTag. The type in the configuration is incorrect. ----- End Fatal Exception ------------------------------------------------- 50101.0 step2 runTheMatrix-results/50101.0_SingleMuPt10+SingleMuPt10FSIdINPUT+SingleMuPt10FS_ID/step2_SingleMuPt10+SingleMuPt10FSIdINPUT+SingleMuPt10FS_ID.log 25.0 step2 runTheMatrix-results/25.0_TTbar+TTbar+DIGI+RECO+HARVEST+ALCATT/step2_TTbar+TTbar+DIGI+RECO+HARVEST+ALCATT.log ----- Begin Fatal Exception 07-Feb-2014 16:38:32 CET----------------------- An exception of category 'Configuration' occurred while [0] Constructing the EventProcessor [1] Validating configuration of module: class=EgammaHLTRegionalPixelSeedGeneratorProducers label='hltL1SeededEgammaRegionalPixelSeedGenerator' Exception Message: Parameter "SeedingLayers" should be defined as a tracked InputTag. The type in the configuration is incorrect. ----- End Fatal Exception ------------------------------------------------- you can see the results of the tests here: |
OK, I fixed up the HIon problem. What is still failing is FastSim.... Any fastsim expert who knows what fastsim does special ----- Begin Fatal Exception 07-Feb-2014 17:27:48 CET----------------------- but maybe this means these modules should be removed for fastsim??? |
It looks to me (based on the current, V28, HLT_8E33v2_Famos_cff.py) that there are many SeedingLayersESProducers defined, but only one used (by ElectronSeedProducer). When doing the configuration migration for RECO I got the impression that fastsim would not use SeedingLayersESProducer. I'd say they could be just removed (except the one used by ElectronSeedProducer). Of course a confirmation from a fastsim expert wouldn't hurt. |
A possible fix for the fastsim issue is in |
@perrotta Andrea, thanks for the suggestion. However, I would like to understand the issue a bit better before including your commit. I took a deeper look on ElectronSeedProducer (again in the context of HLT_8E33v2_Famos_cff.py), and it turns out that all instances of it have If |
@@ -128,14 +126,20 @@ void ElectronSeedProducer::beginRun(edm::Run const&, edm::EventSetup const&) { | |||
SeedFilter::Tokens sf_tokens; | |||
sf_tokens.token_bs = beamSpotTag_; | |||
sf_tokens.token_vtx = filterVtxTag_; | |||
seedFilter_ = new SeedFilter(conf_,sf_tokens) ; | |||
edm::ConsumesCollector iC = consumesCollector(); | |||
seedFilter_.reset(new SeedFilter(conf_, sf_tokens, iC)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SeedFilter is constructed only if preFilteredSeeds==True
.
Hi Andrea, This indeed works! Thanks and best regards Martin On Sat, 8 Feb 2014, perrotta wrote:
Martin Martin W. Gruenewald e-mail: Martin.Grunewald@cern.ch |
Hi Martin and Matti. This also avoids me to check why that HLTPixelTracksProducer worked the same with either two or three layers, great :-) Andrea |
Hi All At present I am not at all familiar with this part of the FastSim code. Cheers Lukas On Mon, Feb 10, 2014 at 9:35 AM, perrotta notifications@github.com wrote:
|
Hi, Great - I'll make the changes/simplifications/ and test again... Best regards Martin On Mon, 10 Feb 2014, perrotta wrote:
Martin Martin W. Gruenewald e-mail: Martin.Grunewald@cern.ch |
Superseded by #2378. |
Fix data files for SLHCUpgradeSimulations/Geometry
SeedingLayers
being constructed by anESProducer
is incompatible with the consumes() interface (they read pixel and stripRecHits
from the event thus needingedm::ConsumesCollector
). Replacing theESProducer
with anEDProducer
has smaller impact on the configuration side than alternatives, and allows some reduction of repeated construction ofTransientTrackingRecHits
from same clusters. In addition to what was presented in the RECO meeting on Jan 16th (https://indico.cern.ch/conferenceDisplay.py?confId=293827), this PR contains further optimization based on comments by Chris and Liz.All configuration files found with
git grep SeedingLayersESProducer
were migrated, except the ones that looked like dumps from ConfDB (underHLTrigger
andDQM/DataScouting
). Because of the configuration changes, all workflows containing HLT will be broken until the changes are migrated to ConfDB and to the dumps.Rebased on top of 7_0_0_pre12 + #2269 (=first two commits).
MultiTrackValidator with 40 events of 13 TeV TTBar + 40 PU (25 ns) gives identical results. With 100 TTbar+40 PU events, overall RECO timing decreases by ~0.5 % and heap size increases by ~10 MB. With 15000 events from run 207515 and menu /online/collisions/2012/8e33/v2.3/HLT/V28 (including prescales), overall HLT timing decreases by ~0.8 % and heap size decreases slightly (~100 kB).
FYI: @Martin-Grunewald, @VinInn