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

migration to stage-2 L1 for muon modules #13572

Merged
merged 8 commits into from Mar 15, 2016

Conversation

sarafiorendi
Copy link
Contributor

Muon specific modules using l1extra collections have been cloned and migrated to new stage-2 L1t collections.

HLTMuonL1Filter → HLTMuonL1TFilter
HLTL1MuonSelector → HLTL1TMuonSelector
HLTMuonL1RegionalFilter → HLTMuonL1TRegionalFilter
HLTMuonTrkFilter → HLTMuonTrkL1TFilter
HLTMuonL1toL3TkPreFilter → HLTMuonL1TtoL3TkPreFilter

HLTMuonL2ToL1Map → HLTMuonL2ToL1TMap
HLTMuonL2PreFilter → HLTMuonL2FromL1TPreFilter
HLTMuonDimuonL2Filter → HLTMuonDimuonL2FromL1TFilter

RecoMuon/L2MuonSeedGenerator/L2MuonSeedGenerator → L2MuonSeedGeneratorFromL1T

An update was also needed for
DataFormats/MuonSeed/interface/L3MuonTrajectorySeed.h
DataFormats/MuonSeed/interface/L2MuonTrajectorySeed.h

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2016

A new Pull Request was created by @sarafiorendi for CMSSW_8_0_X.

It involves the following packages:

DataFormats/MuonSeed
HLTrigger/Muon
RecoMuon/L2MuonSeedGenerator

@perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks.
@battibass, @abbiendi, @jhgoh, @Martin-Grunewald, @bellan, @trocino, @bachtis, @rociovilar 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

@slava77
Copy link
Contributor

slava77 commented Mar 3, 2016

Is the implementation really so different to justify cloned implementation?
It may be better to have updates to existing modules.

On the other hand, this is all just the trigger code.
So, continuing support and bugfixing to the past is going to be limited in time while comparisons are needed between the two and the newer one happens to need an update like a bug fix.

@battibas @jhgoh @HuguesBrun please confirm that this cloning decision was well discussed in muon POG.

<class name="L3MuonTrajectorySeed" ClassVersion="11">

<class name="L3MuonTrajectorySeed" ClassVersion="13">
<version ClassVersion="13" checksum="3249417346"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please change 13 to 12.
We should not skip versions.

@slava77
Copy link
Contributor

slava77 commented Mar 3, 2016

review of cloned code is also more complicated: you will get more comments

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2016

Pull request #13572 was updated. @perrotta, @cmsbuild, @cvuosalo, @fwyzard, @Martin-Grunewald, @deguio, @slava77, @vanbesien, @mulhearn, @davidlange6 can you please check and sign again.

@Martin-Grunewald
Copy link
Contributor

please test

@slava77
Copy link
Contributor

slava77 commented Mar 3, 2016

please clean up the git history from other PR merges.
The last commit dragged in 30K lines of changes.

@slava77
Copy link
Contributor

slava77 commented Mar 3, 2016

-1

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2016

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2016

Pull request #13572 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

@Martin-Grunewald
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2016

@Martin-Grunewald
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2016

@slava77
Copy link
Contributor

slava77 commented Mar 8, 2016

+1

for #13572 46b53dd

  • code changes are OK, although less code replication is desirable
  • jenkins tests pass and comparisons with baseline show no differences

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 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

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Mar 15, 2016
migration to stage-2 L1 for muon modules
@cmsbuild cmsbuild merged commit 28f80aa 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

6 participants