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

Adding new instances of HLTCaloObjInRegionsProducer for HLT filtering #18544

Merged
merged 1 commit into from May 5, 2017

Conversation

Sam-Harper
Copy link
Contributor

This PR adds new templated instances of HLTCaloObjInRegionsProducer to allow it to filter on new types.

This module is used in the HLT to prefilter calo objects (digis currently) to only run computational expensive things on the objects needed. The main use case is running HCAL method 2 for just HCAL hits which interest e/gamma.

The main motivation is to allow it to filter on QE11 HCAL digis for HEP17. At this time I also added in the ability for it to filter ECAL + ES digis incase the time to run multifit globally is to much. Finally I added EcalRecHit and EcalUncalibratedRecHit instances so it could can eventually replace HLTRecHitInAllL1RegionsProducer instances which is a very similar but not as flexable module which is now redundant.

Some minor changes were needed to make the HLTCaloObjInRegionsProducer function with the new objects (front -> begin , wrapping id() in to a DetId as some return ints, some return DetIds).

Finally I also added a function to EB and EE DigiCollections to allow a Digi to be directly pushed back to them like all other Digi collections I've encountered in CMS.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2017

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

It involves the following packages:

DataFormats/EcalDigi
RecoEgamma/EgammaHLTProducers

@cmsbuild, @civanch, @silviodonato, @mdhildreth, @Martin-Grunewald, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks.
@battibass, @rafaellopesdesa, @lgray, @calderona, @HuguesBrun, @argiro this is something you requested to watch as well.
@davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here

@Sam-Harper
Copy link
Contributor Author

Sam-Harper commented May 2, 2017

Oh and I forgot the mention, the testUCTUnpacker in EventFilter/L1TXRawToDigi fails. I think this is because its being called incorrectly, the testUCTUnpacker is expecting input on cin its not getting, ie it should be called like:
$CMSSW_BASE/tmp/$SCRAM_ARCH/src/EventFilter/L1TXRawToDigi/test/testUCTUnpacker/testUCTUnpacker < event_test1.txt

when called like this it does work

@silviodonato
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19528/console Started: 2017/05/02 18:35

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2017

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2017

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 1555 differences found in the comparisons
  • DQMHistoTests: Total files compared: 24
  • DQMHistoTests: Total histograms compared: 1831629
  • DQMHistoTests: Total failures: 21924
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1809525
  • DQMHistoTests: Total skipped: 180
  • DQMHistoTests: Total Missing objects: 0
  • Checked 98 log files, 14 edm output root files, 24 DQM output files

@fwyzard
Copy link
Contributor

fwyzard commented May 3, 2017

looks OK from a quick glance...

@civanch
Copy link
Contributor

civanch commented May 4, 2017

+1

@Martin-Grunewald
Copy link
Contributor

This PR has now become a 92X PR - please make a 91X PR as well.

@Martin-Grunewald
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented May 5, 2017

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

@davidlange6
Copy link
Contributor

+1

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

8 participants