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

[HCAL] Fix rechit flags for R45 filter #23777

Merged
merged 3 commits into from Jul 12, 2018
Merged

Conversation

jaehyeok
Copy link
Contributor

@jaehyeok jaehyeok commented Jul 12, 2018

The HCAL R45 filter algorithm uses two flags: "HBHETS4TS5Noise" and "HBHEOOTPU". Both are set by https://github.com/cms-sw/cmssw/blob/master/RecoLocalCalo/HcalRecAlgos/interface/HBHEPulseShapeFlag.h and it still assumes that SOI is 4.

This PR fixes this issue by getting the SOI from HBHEDataFrame::presamples().

Small changes (increase in rejection rate) are expected in both data and MC in the R45 filter decision.

Following is the rate changes for MET+X triggers measured in JetHT sample (13k events) before and after the fix:

passedFIX/passedBUG HLTPath
0.964613 HLT_CaloMET70_HBHECleaned_v4
0.959184 HLT_CaloMET80_HBHECleaned_v4
0.952408 HLT_CaloMET90_HBHECleaned_v4
0.944777 HLT_CaloMET100_HBHECleaned_v4
0.509202 HLT_CaloMET250_HBHECleaned_v4
0.367521 HLT_CaloMET300_HBHECleaned_v4
0.311828 HLT_CaloMET350_HBHECleaned_v4
0.708633 HLT_PFMET200_HBHECleaned_v9
0.589474 HLT_PFMET250_HBHECleaned_v9
0.513889 HLT_PFMET300_HBHECleaned_v9
0.703557 HLT_PFMET200_HBHE_BeamHaloCleaned_v9
0.763975 HLT_PFMETTypeOne200_HBHE_BeamHaloCleaned_v9
0.998347 HLT_DiJet110_35_Mjj650_PFMET110_v9
0.997959 HLT_DiJet110_35_Mjj650_PFMET120_v9
0.997436 HLT_DiJet110_35_Mjj650_PFMET130_v9
0.810345 HLT_Rsq0p35_v15
0.710526 HLT_Rsq0p40_v15
0.988839 HLT_RsqMR300_Rsq0p09_MR200_v15
0.982788 HLT_RsqMR320_Rsq0p09_MR200_v15
0.992908 HLT_RsqMR300_Rsq0p09_MR200_4jet_v15
0.989583 HLT_RsqMR320_Rsq0p09_MR200_4jet_v15

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@slava77
Copy link
Contributor

slava77 commented Jul 12, 2018

@cmsbuild please test

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 12, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/29080/console Started: 2018/07/12 09:08

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @jaehyeok for master.

It involves the following packages:

RecoLocalCalo/HcalRecAlgos

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@mariadalfonso, @argiro this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2892290
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2892098
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
  • Checked 128 log files, 14 edm output root files, 31 DQM output files

@fabiocos
Copy link
Contributor

@jaehyeok I do not see any significant change reported in the DQM test. Is this due to the limited size of the samples?

@abdoulline
Copy link

abdoulline commented Jul 12, 2018

@fabiocos
While Jae may not yet back to work (in California) - I take the liberty to answer:
there will be a presentation by Mariarosaria (@mariadalfonso) at PPD General today and last slides (No.15-16) devoted to this particular fix:
https://indico.cern.ch/event/739916/contributions/3069851/attachments/1685872/2710999/dalfonso_PPDjuly12.pdf

PR RelVal stat. is too small and not enough specific to see the effect.

@jaehyeok
Copy link
Contributor Author

@fabiocos I think so. The level of changes is in the order of 0.01% in SingleMuon and 1% in JetHT datasets.

@abdoulline
Copy link

@fabiocos @slava77
once 10_2_0 will be cut tomorrow, we need this PR in...

@perrotta
Copy link
Contributor

+1

  • With this PR the position of the sample of interest is not hardcoded in the filter any more, and it can move together with the reduction of digy array from 10 TS to 8 TS (soi from 4 to 3)
    • The same customizability was already implemented in other HCAL code, and it was apparently forgotten here
  • There is no effect visible from jenkins tests, which is kind of expected: "‘negligible’ impact if any of faulty filters" (see M. D'Alfonso, https://indico.cern.ch/event/739916/contributions/3069851/attachments/1685872/2710981/dalfonso_PPDjuly12.pdf)
  • As far as I understand, this has to be validated by TSG in 10_2_0, and a necessary condition for it is that this PR is merged in the release: there are no counterindications from reco

@cmsbuild
Copy link
Contributor

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 will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
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

6 participants