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-gem28 Add a new version of GE11 geometry slice #19190

Merged
merged 1 commit into from Jun 15, 2017

Conversation

bsunanda
Copy link
Contributor

@bsunanda bsunanda commented Jun 11, 2017

This new version reflects the real situation of installed GE1 chambers (in locations 1,27,28,29,30 on -z side). The earlier description was wrong

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

Geometry/CMSCommonData
Geometry/MuonCommonData

@civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @kpedro88, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @ptcox this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@bsunanda
Copy link
Contributor Author

@cmsbuild Please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 11, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20488/console Started: 2017/06/11 20:58

@ianna
Copy link
Contributor

ianna commented Jun 11, 2017

+1

@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-19190/20488/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 22
  • DQMHistoTests: Total histograms compared: 1817858
  • DQMHistoTests: Total failures: 34099
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1783593
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 90 log files, 14 edm output root files, 22 DQM output files

@kpedro88
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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

hi- @bsunanda - is there a summary of the motivations behind the changes in indico?

@bsunanda
Copy link
Contributor Author

@davidlange6 I am not sure. Cesare and Teruki contacted me and it was verified by hardware people. These positions refer to the real positions of chambers installed on CMS last YETS

@davidlange6
Copy link
Contributor

please follow up with them - geometry changes should not come undocumented at this point

@calabria
Copy link
Contributor

@davidlange6 simply the previous geometry was wrong, it did not reflect the chambers we actually installed. In the previous geometry we had E-1 CH19,26,27,28,29 but actually the installed chambers are E-1 CH1,27,28,29,30. Please, have a look here: https://indico.cern.ch/event/641682/contributions/2603235/attachments/1474368/2282957/DPG-GEMMiniWorkShopJune2017.pdf , slides 9, 10 and 11. The current situation is the one that you see in the pic
schermata 2017-06-09 alle 16 28 06
schermata 2017-06-09 alle 16 33 00

ture in attachment.

@davidlange6
Copy link
Contributor

so we've gone from "conforms better" to "wrong". not what I was expecting. Thanks @bsunanda - please update the description as summarized by @calabria in terms of which chambers are installed.

@bsunanda
Copy link
Contributor Author

@davidlange6 Chambers 1, 27, 28, 29, 30 installed on z =-1 side. The new geometry reflects that. I don't understand your comment.

@calabria
Copy link
Contributor

calabria commented Jun 14, 2017

Maybe my comment was not clear: the picures attached to the previous message were for the current situation that is wrong. Instead the picture attached here is obtained with the geometry of this PR, and it clearly shows that the chambers now are correct and consistent with what we installed: endcap -1 and chambers 1,27,28,29,30.
schermata 2017-06-10 alle 15 25 31

@davidlange6
Copy link
Contributor

all is clear already - please edit the description at the top of the PR (eg, the first comment) to reflect what this PR really does rather than a vague comment that implies small changes

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 645c540 into cms-sw:master Jun 15, 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