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

Allowing HLT to be seeded by Taus #14903

Merged
merged 2 commits into from Jun 20, 2016
Merged

Conversation

Sam-Harper
Copy link
Contributor

Dear All,

The L1EG efficiency is poor at high pt and we need to OR with Tau seeds to recover this. This requires the following change to the seeding module.

My small scale test showed that it was correctly matching with taus, now going for a larger scale test

Cheers,
Sam

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

HLTrigger/Egamma

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

@fwyzard
Copy link
Contributor

fwyzard commented Jun 16, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 16, 2016

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

@fwyzard
Copy link
Contributor

fwyzard commented Jun 16, 2016

@Sam-Harper, all, I think I have some general comments about this module - irrespective of the urgent addition of tau candidates to the seeds. Who do you think I should bother with them ?

@Sam-Harper
Copy link
Contributor Author

Yes I agree its a bit nasty. I would talk to Afiq about them.

@fwyzard
Copy link
Contributor

fwyzard commented Jun 16, 2016

+1
assuming it passes the tests and Sam is happy

@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 after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@cmsbuild
Copy link
Contributor

@fwyzard
Copy link
Contributor

fwyzard commented Jun 16, 2016

@Sam-Harper , do you also need a similar change in HLTEcalRecHitInAllL1RegionsProducer ?

@Sam-Harper
Copy link
Contributor Author

Hi Andrea,

I'm happy enough. I get tau matches with this filter. No HLTEcalRecHitInAllL1RegionsProducer is all setup, either Gabi or Afiq had the forsight to do that. I've just proved in fact that module does accept taus .

Cheers,
Sam

@cmsbuild
Copy link
Contributor

@fwyzard
Copy link
Contributor

fwyzard commented Jun 16, 2016 via email

@davidlange6
Copy link
Contributor

Should be ok with 8_0_10 patch. I'll have a look

Sent from my iPad

On Jun 16, 2016, at 6:02 PM, Andrea Bocci <notifications@github.commailto:notifications@github.com> wrote:

Hi David,
could you build an 8.0.x patch release with this PR ?

If it's the same for you, I would prefer an 8.0.10-patch3 , as it would be
easier and faster to switch online.
If that is problematic, for any reasons, then 8.0.11-patch1 will be fine.

Thank you,
.Andrea


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/14903#issuecomment-226531658, or mute the threadhttps://github.com/notifications/unsubscribe/AEzyw0txt5SEL948l0uW9EyU_L6rQvBeks5qMXOEgaJpZM4I3Tpp.

@Martin-Grunewald
Copy link
Contributor

@Sam-Harper
Could you please make a PR for 81X as well?
Thanks!

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit c3d65e8 into cms-sw:CMSSW_8_0_X Jun 20, 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