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

EG L1S1 to L1S2 migration #13558

Merged
merged 1 commit into from Mar 15, 2016

Conversation

Sam-Harper
Copy link
Contributor

@gpasztor

This is the E/gamma migration from stage-1 to stage-2. A lot of the work was also done by Gabriella Pasztor, her work was the base I started from. Because my gitfoo is not strong, I ended up having to lose her commits (it was easier just to copy over the 2 new files and the 2 modified files.

Modules accessing L1 actually in the menu:

HLTEgammaL1MatchFilterRegional:

  • replaced by HLTEgammaL1TMatchFilterRegion

HLTEcalRecHitInAllL1RegionsProducer

  • templated the new stage-2 objects in

EgammaHLTCaloTowerProducer

  • it is my understanding that L1 team will label their collections therefore no code update is strictly necessary.
  • I dont have access to that pull request so in my tests paths with this module failed
  • it will however loop twice on the e/gamma objects which is a bit pointless, will fix in the next update
  • if the L1 team is not going to label their objects, we can provide a new version of this class which works just for the stage-2

Modules not in menu which use L1 info:

HLTEgammaL1MatchFilterPairs:

  • I personally see no use for this module, its not been used in RunII (and possibly most of RunI) so I think we should depreciate it. I would like to actually delete the module but at the very least, we should only update it when people have a use case

HLTRechitInRegionsProducer:

  • this is completely superseeded by HLTRecHitInAllL1RegionsProducer since the start of RunII and has not been used in any RunII menu so depreciate

Configuration changes needed to work the new menu:

  • all instances of cms.Input('hltCaloStage2Digis','Isolated') and cms.Input('hltCaloStage2Digis','NonIsolated') should be changed to cms.Input('hltCaloStage2Digis','EGamma'), assuming L1 names the Egamma collection EGamma
  • hltRechitInRegionsECAL and hltRechitInRegionsES need to have their matching parameters updated. For both l1InputRegions should be (taking into account that the Jet and Egamma collection tags will change)
l1InputRegions = cms.VPSet( 
      cms.PSet(  maxEt = cms.double( 999.0 ),
        regionEtaMargin = cms.double( 0.4 ),
        minEt = cms.double( 5.0 ),
        regionPhiMargin = cms.double( 0.5 ),
        inputColl = cms.InputTag( 'hltCaloStage2Digis' ),
        type = cms.string( "EGamma" )
      ),
      cms.PSet(  maxEt = cms.double( 999.0 ),
        regionEtaMargin = cms.double( 0.4 ),
        minEt = cms.double( 200.0 ),
        regionPhiMargin = cms.double( 0.5 ),
        inputColl = cms.InputTag( 'hltCaloStage2Digis' ),
       type = cms.string( "Jet" )
      )
    ),

Future updates:

We tried to have minimal changes as requested. But we would like to rename the input parameters of HLTEgammaL1MatchFilterRegional to take into account that the L1EGs are not seperated in non-isolated, isolated. We also have to study the new matching requirements at L1. Right now its probably overly large.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2016

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

It involves the following packages:

HLTrigger/Egamma
RecoEgamma/EgammaHLTProducers

@Martin-Grunewald, @perrotta, @cmsbuild, @davidlange6, @fwyzard can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @lgray 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

@Martin-Grunewald
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2016

@Martin-Grunewald
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2016

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2016

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Mar 15, 2016
@cmsbuild cmsbuild merged commit 1f9e458 into cms-sw:CMSSW_8_0_X Mar 15, 2016
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

4 participants