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 distributions over eta and phi #18413

Merged
merged 4 commits into from May 4, 2017

Conversation

natalia-korneeva
Copy link
Contributor

Following last discussions with btagging@HLT conveners the eta/phi dimension's are to be monitored for future campaigns.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 19, 2017

A new Pull Request was created by @natalia-korneeva (Natalia Korneeva) for master.

It involves the following packages:

HLTriggerOffline/Btag

@cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@imarches, @acaudron, @JyothsnaKomaragiri, @mverzett, @ferencek, @pvmulder this is something you requested to watch as well.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here

@natalia-korneeva
Copy link
Contributor Author

New distributions for the 9_0_0_2016 release are available here
https://twiki.cern.ch/twiki/bin/view/Sandbox/NataliaTsirovaSandbox#New_distributions_over_eta_and_p
They are Efficiency vs eta and Efficiency vs phi for b, c, light quarks.

@dmitrijus
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 20, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19281/console Started: 2017/04/20 10:05

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @Muzaffar, @davidlange6, @smuzaffar

@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-18413/19281/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3274 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1826205
  • DQMHistoTests: Total failures: 30110
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1795922
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@@ -46,13 +46,29 @@ HLTBTagHarvestingAnalyzer::dqmEndJob(DQMStore::IBooker & ibooker, DQMStore::IGet
efficsOK[flavour]=isOK;
}
label= m_histoName.at(ind)+std::string("___");
TString labelEta = label;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @natalia-korneeva - as not to make the existing issues with string conversions in this class worse, please change the types of labelEta and labelPhi to be std::string. That will avoid several conversions between string<->Tstring in the code you've added below.

@@ -189,11 +197,19 @@ void HLTBTagPerformanceAnalyzer::bookHistograms(DQMStore::IBooker & ibooker, edm
int nBinsPt=60;
double pTmin=30;
double pTMax=330;
int nBinsPhi=54;
double phimin=-3.14;
Copy link
Contributor

Choose a reason for hiding this comment

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

could you use M_PI here?

@davidlange6
Copy link
Contributor

-1
pending answers to small comments

@cmsbuild
Copy link
Contributor

Pull request #18413 was updated. @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please check and sign again.

@natalia-korneeva
Copy link
Contributor Author

PR updated according to requests.

@mtosi
Copy link
Contributor

mtosi commented Apr 26, 2017

as said, and agreed at the TSG meeting today
please,

  1. add such feature in the code used by the DQM step
  2. add plots for the HEP17, HEM17 and HEP18 slices
  3. add ratio plots between HEP17, HEM17 and HEP18 (probably by using the GenericDQMClient)

@natalia-korneeva
Copy link
Contributor Author

@mtosi, as discussed today at BTV@HLT meeting we propose to leave this PR as it is to be able to monitor the full eta and phi distributions.
The HEP17 etc. plots are to be added with the separate PR as a separate task, this part is in preparation now.

@mtosi
Copy link
Contributor

mtosi commented Apr 27, 2017

thanks for the clarification, hoping it will come soon ;)

@heppye
Copy link
Contributor

heppye commented Apr 27, 2017

@mtosi , thanks for the quick comment and hope this separate PRs plan is fine with release managements. @dmitrijus @davidlange6 , could u please have a look and sign off again? Thanks.

@dmitrijus
Copy link
Contributor

+1

@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/19517/console Started: 2017/05/02 15:56

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2017

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

@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-18413/19517/summary.html

Comparison Summary:

  • You potentially added 110 lines to the logs
  • Reco comparison results: 3319 differences found in the comparisons
  • DQMHistoTests: Total files compared: 24
  • DQMHistoTests: Total histograms compared: 1831629
  • DQMHistoTests: Total failures: 62075
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1769374
  • DQMHistoTests: Total skipped: 180
  • DQMHistoTests: Total Missing objects: 0
  • Checked 98 log files, 14 edm output root files, 24 DQM output files

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 1e992ec into cms-sw:master May 4, 2017
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