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

Run2-hgx192 Complete first level of debugging geometry #26504

Merged
merged 2 commits into from Apr 26, 2019

Conversation

bsunanda
Copy link
Contributor

@bsunanda bsunanda commented Apr 22, 2019

PR description:

Complete first level of debugging V10 geometry of HGCal. There was a bug in finding the layer # from position (affecting only the last layer of the group #28 for EE and #24 or 22 for HE for V9/V10 version). This will surely affect getClosestCell(GlobalPoint) method and applicable to V10 as well as V9 versions.

PR validation:

Tested with the two workflows 24034.0 (D28 geometry) and 29034.0 (D41 geometry)

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

The changes are bug removals and hence applicable to pre3 and pre4 of 10_6_0

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@bsunanda
Copy link
Contributor Author

@cmsbuild Please test

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26504/9365

  • This PR adds an extra 48KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 22, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/34295/console Started: 2019/04/22 20:25

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

Geometry/CaloTopology
Geometry/HGCalCommonData
Geometry/HGCalGeometry
SimG4CMS/Calo

@civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @kpedro88 can you please review it and eventually sign? Thanks.
@makortel 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-26504/34295/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3074 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3211964
  • DQMHistoTests: Total failures: 4332
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3207428
  • DQMHistoTests: Total skipped: 204
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@cvuosalo
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

Pull request #26504 was updated. @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @kpedro88 can you please check and sign again.

@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-26504/34323/summary.html

There are some workflows for which there are errors in the baseline:
1001.0 step 2
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3070 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3211870
  • DQMHistoTests: Total failures: 4332
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3207334
  • DQMHistoTests: Total skipped: 204
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 130 log files, 14 edm output root files, 32 DQM output files

@cvuosalo
Copy link
Contributor

+1

@kpedro88
Copy link
Contributor

@bsunanda please clarify exactly which bugs were fixed. I see comparison differences not just in D41, but also in D35, which was used for the MTD TDR and in principle should not be affected by changes to the HGCal v10 geometry.

@bsunanda
Copy link
Contributor Author

@kpedro88 Yes there was a bug in getCLosestCell which will affect V9 as well as V10 geometry. This is expected.

@bsunanda
Copy link
Contributor Author

The bug was affecting the last layer of a given setup (layer 28 for EE and layer# 24 or 22 for HE depending on the HGCal version #). I prefer to have bug corrected code in the repository. MTD production was made with earlier CMSSW version and they can continue with that version for MTD analysis.

@kpedro88
Copy link
Contributor

@bsunanda thanks, this is acceptable as long as there is a legitimate, understood reason for the change. Can you update the PR description with these details?

@civanch
Copy link
Contributor

civanch commented Apr 25, 2019

+1

@kpedro88
Copy link
Contributor

+upgrade

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

@kpedro88
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 64aea4c into cms-sw:master Apr 26, 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

5 participants