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

[RecoTauTag] migration to stage-2 L1 #13480

Merged

Conversation

rmanzoni
Copy link
Contributor

Tau specific modules that were using old legacy/stage-1 l1extra collections have been cloned and migrated to new stage-2 l1t collections [*]:

  • L1HLTJetsMatchingL1THLTJetsMatching (this is was used with the Run1 system but not with stage-1, shall we get rid of it?)
  • L1HLTTauMatchingL1THLTTauMatching
  • CaloTowerCreatorForTauHLTCaloTowerFromL1TCreatorForTauHLT

[*]
we followed the instructions in
https://docs.google.com/document/d/1GnnWAaP1C-dbMreC72XCJ5ToQZ55Vfy-O7X_ON5GYjs/edit#
and
https://hypernews.cern.ch/HyperNews/CMS/get/hlt/4171/1.html

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @rmanzoni (Riccardo Manzoni) for CMSSW_8_0_X.

It involves the following packages:

RecoTauTag/HLTProducers

@Martin-Grunewald, @perrotta, @cmsbuild, @davidlange6, @fwyzard can you please review it and eventually sign? Thanks.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

l1t::JetVectorRef jetCandRefVec;
l1t::JetVectorRef objL1CandRefVec;
l1t::JetRef tauCandRef;

Copy link
Contributor

Choose a reason for hiding this comment

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

Taus at stage-2 have their own class type, the jet class is not reused (as it was in stage-1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this module is not used anymore (since the advent of stage-1 I think) and I couldn't find it in the latest menus.
I would be for getting rid of it, if you agree.
Otherwise I'll fix it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removal is fine with me!

Copy link
Contributor

@fwyzard fwyzard Feb 25, 2016 via email

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, then I leave the old L1HLTJetsMatching (w/o 'T') but will not add it's stage-2 sibling L1THLTJetsMatching (w/ 'T')

@Martin-Grunewald
Copy link
Contributor

Why do you actually need to change L1HLTTauMatching to L1THLTTauMatching?

@rmanzoni
Copy link
Contributor Author

perhaps the change is not fundamental, in the sense that this module is not reading any collection spat out by the L1, but internally it uses types which live under l1extra namespace, e.g.
https://github.com/cms-sw/cmssw/blob/CMSSW_8_1_X/RecoTauTag/HLTProducers/src/L1HLTTauMatching.cc#L48
which may be wiped away at some point (will it?)

@Martin-Grunewald
Copy link
Contributor

right, ok, so we need the L1THLTTauMatching

@cmsbuild
Copy link
Contributor

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

aDesc.addUntracked<int> ("verbose" , 0 )->setComment("Verbosity level; 0=silent" );
aDesc.addUntracked<int> ("BX" , 0 )->setComment("Set bunch crossing; 0 = in time, -1 = previous, 1 = following");

desc.add ("caloTowerMakerHLT", aDesc);
Copy link
Contributor

Choose a reason for hiding this comment

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

The string should be the class name, at most starting with a lower-case 'c'.
Also to be different from the one in the old module!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean something like:
desc.add ("caloTowerFromL1TMakerHLT", aDesc);

@cmsbuild
Copy link
Contributor

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


CaloTowerFromL1TCreatorForTauHLT::CaloTowerFromL1TCreatorForTauHLT( const ParameterSet & p )
:
mBX (p.getUntrackedParameter<int> ("BX" , 0) ),
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes physics so must be tracked.

@cmsbuild
Copy link
Contributor

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

@Martin-Grunewald
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

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

@Martin-Grunewald
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

@Martin-Grunewald
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

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

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Mar 1, 2016
…gration

[RecoTauTag] migration to stage-2 L1
@cmsbuild cmsbuild merged commit 5c54649 into cms-sw:CMSSW_8_0_X Mar 1, 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

5 participants