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

[HGCal] Correcting x-axis in some HGCalValidator plots and layer numbering in V10 #26909

Merged
merged 3 commits into from May 28, 2019

Conversation

apsallid
Copy link
Contributor

PR description:

We spotted (@felicepantaleo , @rovere ) two issues in the HGCalValidator and try to address them in this PR:

  1. Efficiency/Duplicates/Fakes/Merges vs eta,phi per layer plots had wrong x-axis labels.
  2. A modifier is introduced to correctly set the total number of layers in case of the HGCal V10 geometry which has two layers less that V9.

PR validation:

To test the changes we created plots for 24005.0 workflow in this link

if this PR is a backport please specify the original PR:

This is not a backport.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26909/9952

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

Validation/HGCalValidation

@andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks.
@vandreev11, @sethzenz, @rovere, @lgray, @cseez, @bsunanda, @pfs, @kpedro88 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

@jfernan2
Copy link
Contributor

Hi @apsallid
We don't have you identifed as DQM developer in:
https://twiki.cern.ch/twiki/bin/viewauth/CMS/DQMContacts#HGCAL
Could you please add yourself in the corresponding e-group for future references?
Thanks

@apsallid
Copy link
Contributor Author

Hi @jfernan2
Ok, I just add myself to the e-group.
Thanks.

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 24, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/479/console Started: 2019/05/24 10:04

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@felicepantaleo
Copy link
Contributor

@apsallid many thanks for the PR.
Could you profit from this PR to move makeHGCalValidationPlots.py from python to scripts folder and check that it works out of the box with v10?

@apsallid
Copy link
Contributor Author

@felicepantaleo
Unfortunately, the makeHGCalValidationPlots.py won't work out of the box. In hgcalPlots.py I deliberately put these lines here as @kpedro88 suggested, in order to avoid accidental use of the code for one geometry but different layerscheme geometry values. As Kevin said, when this info will be in CMSSW then it will automatically fill the GeneralInfo directory. I hope I haven't miss any development there.

@felicepantaleo
Copy link
Contributor

felicepantaleo commented May 24, 2019

@apsallid I don't understand. Instead of getting the layer scheme from a default named DQM file, you can simply get it from the first DQM file passed to the python script.
If I had to choose if I wanted to make it work with the latest geometry or for the old one, I would choose the new geometry.

@apsallid
Copy link
Contributor Author

@felicepantaleo OK, if I can get a confirmation from reviewers and @kpedro88 I can use the latest V10 values and put the script under scripts.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3206516
  • DQMHistoTests: Total failures: 2289
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3203893
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -1396.112 KiB( 32 files compared)
  • DQMHistoSizes: changed ( 27434.0,... ): -2.148 KiB HGCAL/HGCalValidator
  • DQMHistoSizes: changed ( 29034.0 ): -1389.668 KiB HGCAL/HGCalValidator
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@apsallid
Copy link
Contributor Author

@kpedro88 @felicepantaleo ,
Thanks a lot, I applied the changes.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26909/9991

  • This PR adds an extra 36KB to repository

@cmsbuild
Copy link
Contributor

Pull request #26909 was updated. @andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please check and sign again.

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 26, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/520/console Started: 2019/05/26 22:10

@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-12ddb4/520/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3208892
  • DQMHistoTests: Total failures: 2650
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3205908
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -1396.112 KiB( 32 files compared)
  • DQMHistoSizes: changed ( 20034.0,... ): -2.148 KiB HGCAL/HGCalValidator
  • DQMHistoSizes: changed ( 29034.0 ): -1389.668 KiB HGCAL/HGCalValidator
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@jfernan2
Copy link
Contributor

@apsallid do we understand why for some 14TeV workflows only 104 histograms change while for 29034.0 wf almost 2k are modified?
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_0_X_2019-05-26-0000+12ddb4/31917/dqm-histo-comparison-summary.html
Is this expected?

@apsallid
Copy link
Contributor Author

@jfernan2
Yes, this is because for 29034.0 wf the V10 modifier is activated and the total number of layers changes.

@jfernan2
Copy link
Contributor

+1

@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

@cmsbuild cmsbuild merged commit 582fce1 into cms-sw:master May 28, 2019
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