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

Updating E/gamma supercluster/ecal candidate filtering tools to allow future jetcore seeding by e/gamma #21818

Merged
merged 4 commits into from Feb 1, 2018

Conversation

Sam-Harper
Copy link
Contributor

Dear All,

The PR allows the jet core regional tracking to be seeded by E/gamma objects, specifically superclusters with Et> 20 GeV and H/E <0.2.

Currently jet core regional tracking is seeded by 100 GeV CaloJets. This causes problems for E/gamma as electrons/photons will also be reconstructed as 100 GeV CaloJets and so causes the track isolation defination to change at 100 GeV which has caused problems in the past. It also means that the variable which is studied and validated at the Z peak is different to the variable used at high pt which is far from ideal. Therefore we now run this in the region for all e/gamma objects.

A pleasant bonus is that it is likely that this will improve the performance of tracker based isolation for e/gamma. It was shown slightly over a year ago that this is true for high energy electrons so could be true for < 100 GeV electrons too https://indico.cern.ch/event/491518/contributions/2363233/attachments/1367167/2071567/trkIsolHEEP_EGMIDV2.pdf (slides 12-14, compare the black line with the dotted black line).
Unfortunately E/gamma gave me some erroneous information then that jet core regional was being removed in 2017 otherwise this PR would have happened last year...

This PR leverages RecoEcalCandidates to convert superclusters into something inheriting from candidates, using existing well established HLT code. Not to re-invent the wheel, I reused EgammaHLTFilteredSuperClusterProducer to filter the RecoEcalCandidates. This mean I had to turn it into a generic templated class EgammaHLTFilteredObjProducer and then have instances of that to produce a std::vector of edm::Ptrs.

I then combine the selected jets (which are a collection of edm::Ptrreco::Candidate with the selected e/gammas using a new class CandidateCombiner to make a new collection egammaAndJetsForCoreTracking which seeds the tracking regions. The jets and e/gammas are loosely dR cleaned (ie they have tightish dR match) with e/gammas eta/phi being preferred.

Validation:
In CMSSW_10_0_0_pre3 I re-recoed a file from /RelValTTbar_13/CMSSW_10_0_0_pre3-PU25ns_100X_upgrade2018_realistic_v4_mahiOFF-v1/GEN-SIM-DIGI-RAW (/store/relval/CMSSW_10_0_0_pre3/RelValTTbar_13/GEN-SIM-DIGI-RAW/PU25ns_100X_upgrade2018_realistic_v4_mahiOFF-v1/20000/048FD543-85E9-E711-8FC9-0CC47A78A414.root, 500 events)

with the cmsDriver recoTiming.py step3 --conditions auto:phase1_2018_realistic -n 10 --era Run2_2018 --customise_commands process.hbheprereco.algorithm.useMahi = cms.bool(False) \n process.hbheprereco.algorithm.useM2 = cms.bool(True) \n --runUnscheduled -s RAW2DIGI,L1Reco,RECO,RECOSIM,EI,PAT --datatier GEN-SIM-RECO,MINIAODSIM --eventcontent RECOSIM,MINIAODSIM --geometry DB:Extended --no_exec --filein file:step2.root --fileout file:step3.root --nThreads 4 --no_exec

with both this PR and clean CMSSW_10_0_0_pre3:

default pre3 seconds per event: 16.99 , 17.20, 16.93, 17.47
with this PR: 17.47, 17.13, 17.49, 16.99.

Looking at detail for the output comparing the fastest "default" one and the slowest one with this pr:
Couldn't parse time report using CPU and wall-clock format: trying Wall-clock only
delta/mean delta/orJob original new module name


   added      +0.00%         0.00 ms/ev ->         0.30 ms/ev egammaHoverEForTrk
   added      +0.00%         0.00 ms/ev ->         0.02 ms/ev egammasForTrk
   added      +0.00%         0.00 ms/ev ->         0.01 ms/ev egammasForCoreTracking
   added      +0.02%         0.00 ms/ev ->         3.04 ms/ev particleFlowSuperClusterECALForTrk
   added      +0.00%         0.00 ms/ev ->         0.02 ms/ev egammaAndJetsForCoreTracking

+1.971550 +0.47% 0.58 ms/ev -> 80.41 ms/ev dt1DRecHits
+1.969100 +0.54% 0.72 ms/ev -> 92.23 ms/ev l1extraParticles
+1.955702 +0.05% 0.09 ms/ev -> 8.39 ms/ev dt1DCosmicRecHits
+1.944751 +0.05% 0.12 ms/ev -> 8.57 ms/ev castorreco
-1.909900 -0.02% 3.51 ms/ev -> 0.08 ms/ev jetsForCoreTrackingPreSplitting
+1.907162 +0.03% 0.11 ms/ev -> 4.42 ms/ev gtDigis
-1.891304 -0.02% 3.40 ms/ev -> 0.10 ms/ev jetsForCoreTracking
-1.812378 -0.22% 39.18 ms/ev -> 1.93 ms/ev siStripZeroSuppression
+1.808692 +0.52% 4.63 ms/ev -> 92.22 ms/ev ecalPreshowerRecHit
+1.793510 +0.00% 0.03 ms/ev -> 0.64 ms/ev offlineBeamSpot
-1.773824 -0.03% 4.77 ms/ev -> 0.29 ms/ev totemRPRawToDigi
-1.738878 -0.01% 1.93 ms/ev -> 0.14 ms/ev corrPfMetType1Puppi
+1.709287 +0.15% 2.10 ms/ev -> 26.82 ms/ev displacedGlobalMuons
-1.665399 -0.01% 2.41 ms/ev -> 0.22 ms/ev caloJetMap
-1.623188 -0.02% 3.38 ms/ev -> 0.35 ms/ev corrPfMetType1NoHF
-1.571429 -0.00% 0.07 ms/ev -> 0.01 ms/ev ak4PFCHSL3AbsoluteCorrector
+1.527711 +0.00% 0.05 ms/ev -> 0.37 ms/ev ak4PFCHSL2RelativeCorrector
+1.514061 +0.00% 0.11 ms/ev -> 0.78 ms/ev ak4PFPuppiResidualCorrector
+1.481481 +0.00% 0.03 ms/ev -> 0.19 ms/ev hpsPFTauDiscriminationByMVA6VTightElectronRejection
+1.445545 +0.00% 0.03 ms/ev -> 0.17 ms/ev hpsPFTauDiscriminationByMVA6MediumElectronRejection
+1.440242 +0.01% 0.18 ms/ev -> 1.14 ms/ev patJetCorrFactorsAK8
+1.391304 +0.00% 0.03 ms/ev -> 0.16 ms/ev hpsPFTauDiscriminationByMVA6TightElectronRejection
-1.384111 -0.00% 1.03 ms/ev -> 0.19 ms/ev pfCombinedSecondaryVertexV2BJetTagsAK8Puppi
+1.328467 +0.00% 0.02 ms/ev -> 0.11 ms/ev hpsPFTauDiscriminationByLooseIsolationMVArun2v1PWdR03oldDMwLT
+1.318386 +0.00% 0.04 ms/ev -> 0.18 ms/ev hpsPFTauDiscriminationByMVA6VLooseElectronRejection
-1.283105 -0.00% 0.72 ms/ev -> 0.16 ms/ev patPFMetT1T2CorrNoHF
-1.268855 -0.02% 4.60 ms/ev -> 1.03 ms/ev ctppsPixelLocalTracks
+1.225251 +0.00% 0.14 ms/ev -> 0.56 ms/ev castorDigis
-1.208270 -0.01% 2.64 ms/ev -> 0.65 ms/ev muonSeededSeedsOutInDisplaced
+1.164694 +0.14% 8.45 ms/ev -> 32.02 ms/ev gedPhotons
+1.140787 +0.01% 0.68 ms/ev -> 2.49 ms/ev initialStepSeedLayersPreSplitting
+1.112903 +0.00% 0.11 ms/ev -> 0.39 ms/ev patJetCorrFactorsAK8Puppi
+1.102111 +0.15% 10.43 ms/ev -> 36.04 ms/ev globalMuons
+1.080904 +0.02% 1.19 ms/ev -> 3.99 ms/ev pfCombinedSecondaryVertexV2BJetTags
-1.077056 -0.01% 1.79 ms/ev -> 0.54 ms/ev corrPfMetType1
+1.045872 +0.00% 0.23 ms/ev -> 0.75 ms/ev softPFMuonBJetTags
-1.030189 -0.00% 0.80 ms/ev -> 0.26 ms/ev patPFMetT1T2Corr
+0.991511 +0.00% 0.30 ms/ev -> 0.88 ms/ev pfBoostedDoubleSecondaryVertexAK8BJetTagsAK8Puppi
-0.966956 -0.02% 4.02 ms/ev -> 1.40 ms/ev muonSeededSeedsInOut
-0.953405 -0.01% 3.30 ms/ev -> 1.17 ms/ev pfCombinedCvsLJetTagsPuppi
-0.951872 -0.16% 42.15 ms/ev -> 14.97 ms/ev csc2DRecHits
+0.937513 +0.01% 1.27 ms/ev -> 3.51 ms/ev MeasurementTrackerEventPreSplitting
-0.930900 -0.00% 1.12 ms/ev -> 0.41 ms/ev basicJetsForMetPuppi
-0.785068 -0.00% 1.23 ms/ev -> 0.54 ms/ev basicJetsForMetNoHF
+0.750000 +0.00% 0.38 ms/ev -> 0.84 ms/ev hpsPFTauDiscriminationByMVA6rawElectronRejection
-0.745278 -0.01% 2.69 ms/ev -> 1.23 ms/ev QGTagger
+0.708428 +0.05% 8.10 ms/ev -> 16.99 ms/ev pfCombinedCvsLJetTags
-0.697828 -0.01% 3.26 ms/ev -> 1.57 ms/ev basicJetsForMet
-0.653978 -0.01% 1.74 ms/ev -> 0.88 ms/ev pfCombinedCvsBJetTagsPuppi
+0.625937 +0.00% 0.60 ms/ev -> 1.14 ms/ev jetCoreRegionalStep
-0.613397 -0.01% 2.17 ms/ev -> 1.15 ms/ev patSmearedJets
-0.559363 -0.00% 1.61 ms/ev -> 0.90 ms/ev oniaPhotonCandidates
-0.505392 -0.01% 2.15 ms/ev -> 1.28 ms/ev shiftedPatJetResUp
+0.492910 +0.00% 0.56 ms/ev -> 0.92 ms/ev jetCoreRegionalStepHitDoublets
-0.466454 -0.00% 1.54 ms/ev -> 0.96 ms/ev shiftedPatJetResUpNoHF
-0.464340 -0.00% 0.81 ms/ev -> 0.51 ms/ev muonSeededSeedsOutIn
+0.446649 +0.02% 6.73 ms/ev -> 10.61 ms/ev hcalDigis
-0.444502 -0.01% 4.76 ms/ev -> 3.03 ms/ev CHSCands
+0.436803 +0.00% 0.84 ms/ev -> 1.31 ms/ev hpsPFTauDiscriminationByIsolationMVArun2v1PWnewDMwLTraw
-0.430425 -0.00% 1.36 ms/ev -> 0.88 ms/ev patSmearedJetsNoHF
+0.418327 +0.00% 0.79 ms/ev -> 1.21 ms/ev hpsPFTauDiscriminationByIsolationMVArun2v1PWdR03oldDMwLTraw
+0.390480 +0.01% 3.42 ms/ev -> 5.07 ms/ev jetCoreRegionalStepTracks
-0.381050 -0.01% 4.05 ms/ev -> 2.75 ms/ev cosmicDCSeeds
+0.364011 +0.01% 2.58 ms/ev -> 3.73 ms/ev pfGhostTrackBJetTags
+0.343741 +0.37% 151.63 ms/ev -> 214.57 ms/ev jetCoreRegionalStepTrackCandidates
-0.338765 -0.00% 1.18 ms/ev -> 0.84 ms/ev shiftedPatSmearedJetResUpNoHF
+0.333570 +0.01% 5.87 ms/ev -> 8.21 ms/ev pfCombinedCvsBJetTags
-0.312187 -0.01% 6.69 ms/ev -> 4.88 ms/ev patPFMet
+0.302409 +0.00% 0.83 ms/ev -> 1.12 ms/ev hpsPFTauDiscriminationByIsolationMVArun2v1DBdR03oldDMwLTraw
+0.294889 +0.04% 17.39 ms/ev -> 23.40 ms/ev initialStepHitQuadrupletsPreSplitting
-0.285942 -0.00% 1.43 ms/ev -> 1.07 ms/ev shiftedPatJetResDown
+0.268191 +0.01% 3.11 ms/ev -> 4.07 ms/ev pfCombinedMVAV2BJetTags
+0.239212 +0.01% 7.51 ms/ev -> 9.55 ms/ev siPixelDigis
-0.237168 -0.00% 0.63 ms/ev -> 0.50 ms/ev interestingGedEleIsoDetIdEE
-0.232258 -0.00% 1.04 ms/ev -> 0.82 ms/ev shiftedPatSmearedJetResDownNoHF
-0.216127 -0.01% 6.38 ms/ev -> 5.14 ms/ev patMETs
+0.202348 +0.00% 0.80 ms/ev -> 0.98 ms/ev hpsPFTauDiscriminationByIsolationMVArun2v1PWoldDMwLTraw
+0.202279 +0.00% 0.63 ms/ev -> 0.77 ms/ev tripletElectronSeedLayers
-0.158194 -0.02% 18.79 ms/ev -> 16.03 ms/ev initialStepSeedsPreSplitting
+0.143703 +0.07% 73.33 ms/ev -> 84.69 ms/ev ecalMultiFitUncalibRecHit
+0.142177 +0.02% 19.90 ms/ev -> 22.95 ms/ev CSCHaloData
+0.115197 +0.01% 12.67 ms/ev -> 14.22 ms/ev standAloneMuons
+0.114189 +0.01% 11.42 ms/ev -> 12.80 ms/ev particleFlowClusterOOTECAL
+0.108277 +0.01% 11.23 ms/ev -> 12.52 ms/ev particleFlowClusterECAL
+0.101760 +0.01% 13.35 ms/ev -> 14.78 ms/ev siPixelClusters
+0.089352 +0.02% 31.98 ms/ev -> 34.97 ms/ev trackVertexArbitrator
+0.089106 +0.00% 7.34 ms/ev -> 8.03 ms/ev muonCSCDigis
+0.086487 +0.01% 25.14 ms/ev -> 27.42 ms/ev generalV0Candidates
+0.082014 +0.44% 864.36 ms/ev -> 938.28 ms/ev hbheprereco
+0.063157 +0.19% 496.49 ms/ev -> 528.87 ms/ev initialStepTrackCandidatesPreSplitting
+0.060664 +0.03% 93.83 ms/ev -> 99.70 ms/ev siPixelClustersPreSplitting
+0.059434 +0.01% 36.81 ms/ev -> 39.07 ms/ev candidateVertexArbitrator
+0.058152 +0.02% 48.39 ms/ev -> 51.28 ms/ev candidateVertexArbitratorCvsL
+0.050909 +0.00% 5.76 ms/ev -> 6.06 ms/ev detachedQuadStep


Job total: 16.9326 s/ev ==> 17.4926 s/ev
the time increase is due to completely unrelated modules which this PR can not effect, eg hbheprereco, ecalPreshowerRecHit

Looking at the relavent module, the main driver of actual changes are jetCoreRegionalStepTrackCandidates which does from 153.5,153.2, 152.23,151.6 -> 210.5, 207.4, 206.4, 214.6 so a 0.35% increase.

In general we get at most one or two extra tracks every so often so no diskspace concerns.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 9, 2018

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 9, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 9, 2018

A new Pull Request was created by @Sam-Harper for master.

It involves the following packages:

RecoEgamma/EgammaHLTProducers
RecoEgamma/EgammaPhotonProducers
RecoEgamma/EgammaTools
RecoTracker/IterativeTracking

@perrotta, @cmsbuild, @silviodonato, @slava77, @Martin-Grunewald, @fwyzard can you please review it and eventually sign? Thanks.
@ghellwig, @varuns23, @battibass, @makortel, @felicepantaleo, @jainshilpi, @GiacomoSguazzoni, @rovere, @lgray, @calderona, @HuguesBrun, @mschrode, @drkovalskyi, @gpetruc, @ebrondol, @VinInn, @dgulhan, @folguera this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@davidlange6
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 9, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/25348/console Started: 2018/01/09 18:44

@slava77
Copy link
Contributor

slava77 commented Jan 9, 2018

@Sam-Harper
it would be nice to check what is happening in JetHT and DoubleEG

I'm not sure which machine you were using for tests. It could be that a single thread test will give more stable timing and will also allow easily to skip the first event contribution as well.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 9, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 9, 2018

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 9, 2018

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21818/25348/summary.html

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

  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-21818/1325.7_TTbar_13_94XNanoAODINPUT+TTbar_13_94XNanoAODINPUT+NANOEDMMC2017

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3218 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2775243
  • DQMHistoTests: Total failures: 6582
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2768491
  • DQMHistoTests: Total skipped: 169
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0 KiB( 0 files compared)
  • Checked 110 log files, 9 edm output root files, 26 DQM output files

@Sam-Harper
Copy link
Contributor Author

@slava77 , okay will run on some JetHET and DoubleEG data, I'll pick a suitable lumi leveled period. Will get it done asap, should by the end of the day. And I'll run the jobs single threaded. I'm running on the local RAL machines as its easier, is there a particular benchmark machine you would like me to run on instead (eg like the HLT runs on vocms003/004.)

@slava77
Copy link
Contributor

slava77 commented Jan 10, 2018 via email

@Martin-Grunewald
Copy link
Contributor

+1

@Martin-Grunewald
Copy link
Contributor

+1

@slava77
Copy link
Contributor

slava77 commented Jan 30, 2018

+1

for #21818 8f3353d

  • infrastructure to select interesting clusters for a possible inclusion in the jetCore. The default jetCore is unchanged; pending better understanding of what to do with the excessive timing.
  • jenkins tests pass and comparisons with the baseline show no differences

@Sam-Harper
please update the PR description (and perhaps the PR title as well) to reflect what's actually changed.

@Sam-Harper Sam-Harper changed the title Allowing E/gamma objects to seed jet core regional tracking Updating E/gamma supercluster/ecal candidate filtering tools to allow future jetcore seeding by e/gamma Jan 31, 2018
@fabiocos
Copy link
Contributor

fabiocos commented Feb 1, 2018

+1

@perrotta
Copy link
Contributor

perrotta commented Feb 1, 2018

@fabiocos : this PR still misses the Analysis signature. If you want to merge and bypass that signature I think you must write "merge" explicitly

@fabiocos
Copy link
Contributor

fabiocos commented Feb 1, 2018

merge

@cmsbuild cmsbuild merged commit 46354df into cms-sw:master Feb 1, 2018
@Martin-Grunewald
Copy link
Contributor

Martin-Grunewald commented Mar 15, 2018

@Sam-Harper
I am afraid there is a problem with this PR as the three plugins defined do NOT generate a cfi file from their fillDescriptions. It seems the reason is that using is used, instead of typedef, when declaring the actual type and putting this name into the FWK macro.

using EgammaHLTFilteredEcalCandProducer=EgammaHLTFilteredObjProducer<reco::RecoEcalCandidateCollection>;
DEFINE_FWK_MODULE(EgammaHLTFilteredEcalCandProducer);

using EgammaHLTFilteredEcalCandPtrProducer=EgammaHLTFilteredObjProducer<std::vector<edm::Ptr<reco::Candidate> > >;
DEFINE_FWK_MODULE(EgammaHLTFilteredEcalCandPtrProducer);

using EgammaHLTFilteredSuperClusterProducer=EgammaHLTFilteredObjProducer<std::vector<reco::SuperClusterRef>>;
DEFINE_FWK_MODULE(EgammaHLTFilteredSuperClusterProducer);

@Sam-Harper
Copy link
Contributor Author

oh joy, thanks @Martin-Grunewald. Will update it to be typedef but later today not now.

@makortel
Copy link
Contributor

Umm, using should equivalent to typedef.

@fwyzard
Copy link
Contributor

fwyzard commented Mar 15, 2018

Agreed...

@makortel
Copy link
Contributor

(or, for language lawyers, if not exactly equivalent, functionally equivalent for this case)

@Sam-Harper
Copy link
Contributor Author

thats what I thought, and I thought one should always use using in preference to typedef now. So very odd, will look into it later, no time right now

@makortel
Copy link
Contributor

thats what I thought, and I thought one should always use using in preference to typedef now

Indeed, using is the C++11 way (allowing also alias templates).

@Martin-Grunewald
Copy link
Contributor

Uhmm OK, I see two, EgammaHLTFilteredEcalCandPtrProducer and EgammaHLTFilteredEcalCandProducer, with generated cfi files

egammaHLTFilteredObjProducerStdVectorEdmPtrRecoCandidateStdAllocatorEdmPtrRecoCandidate_cfi.py
egammaHLTFilteredObjProducerStdVectorRecoRecoEcalCandidateStdAllocatorRecoRecoEcalCandidate_cfi.py

but the third is still missing...

@Martin-Grunewald
Copy link
Contributor

So I addpkg the package and re-compile it. When it gets to this step, it seems to generate all three:

@@@@ Running edmWriteConfigs for RecoEgammaEgammaHLTProducers
egammaHLTFilteredObjProducerStdVectorEdmRefStdVectorRecoSuperClusterStdAllocatorRecoSuperClusterRecoSuperClusterEdmRefhelperFindUsingAdvanceStdVectorRecoSuperClusterStdAllocatorRecoSuperClusterRecoSuperClusterStdAllocatorEdmRefStdVectorRecoSuperClusterStdAllocatorRecoSuperClusterRecoSuperClusterEdmRefhelperFindUsingAdvanceStdVectorRecoSuperClusterStdAllocatorRecoSuperClusterRecoSuperCluster
hltEgammaHLTPixelMatchVarProducer
hltCaloObjInRegionsProducerHBHEDataFrameEdmSortedCollectionHBHEDataFrameEdmStrictWeakOrderingHBHEDataFrame
[....]
hltRechitInRegionsProducerRecoRecoChargedCandidate
egammaHLTFilteredObjProducerStdVectorRecoRecoEcalCandidateStdAllocatorRecoRecoEcalCandidate
egammaHLTFilteredObjProducerStdVectorEdmPtrRecoCandidateStdAllocatorEdmPtrRecoCandidate

but then in the cfi directory, only two are available:

ls $CMSSW_BASE/cfipython/slc6_amd64_gcc630/RecoEgamma/EgammaHLTProducers/eg**py
/data/CMS1/CMSSW_10_1_X_2018-03-13-2300/cfipython/slc6_amd64_gcc630/RecoEgamma/EgammaHLTProducers/egammaHLTFilteredObjProducerStdVectorEdmPtrRecoCandidateStdAllocatorEdmPtrRecoCandidate_cfi.py
/data/CMS1/CMSSW_10_1_X_2018-03-13-2300/cfipython/slc6_amd64_gcc630/RecoEgamma/EgammaHLTProducers/egammaHLTFilteredObjProducerStdVectorRecoRecoEcalCandidateStdAllocatorRecoRecoEcalCandidate_cfi.py

Maybe the third is lost due to its overly long name?

@wddgit @Dr15Jones
is there a file name length limitation?

@makortel
Copy link
Contributor

I suspect the problem with the third one is that the module label becomes egammaHLTFilteredObjProducerStdVectorEdmRefStdVectorRecoSuperClusterStdAllocatorRecoSuperClusterRecoSuperClusterEdmRefhelperFindUsingAdvanceStdVectorRecoSuperClusterStdAllocatorRecoSuperClusterRecoSuperClusterStdAllocatorEdmRefStdVectorRecoSuperClusterStdAllocatorRecoSuperClusterRecoSuperClusterEdmRefhelperFindUsingAdvanceStdVectorRecoSuperClusterStdAllocatorRecoSuperClusterRecoSuperCluster which has ~390 characters (we're still limited to 255 characters, right?)

@Sam-Harper Easiest way to get shorter auto-generated name would be to use

descriptions.addWithDefaultLabel(desc);

instead of

descriptions.add(defaultModuleLabel<EgammaHLTFilteredObjProducer<OutCollType>>(),desc);

in EgammaHLTFilteredObjProducer<OutCollType>::fillDescription(). addWithDefaultLabel is a recent addition, and derives the module label from the plugin name (what is given to DEFINE_FWK_MODULE()) instead of the full C++ class name.

I'll also take a look why edmWriteConfigs didn't complain.

@Sam-Harper
Copy link
Contributor Author

Yep, I've already been using that happily in other modules, its a much appreciated change!

@makortel
Copy link
Contributor

@Dr15Jones @wddgit

I'll also take a look why edmWriteConfigs didn't complain.

It didn't complain because there was no check whether opening the file for writing succeeded or not. I'll add a check and make a PR (and then we'll see how many things break).

@Martin-Grunewald
Copy link
Contributor

#22625 fixed the plugin with addWithDefaultLabel as this then produces all three files.
It does not contain any fixes/checks on the cfi handling by the build code.

@makortel
Copy link
Contributor

#22626 adds a check for file open errors to edmWriteConfigs.

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