GE0 geometry builder #29624

watson-ij · 2020-05-04T09:45:38Z

PR description:

Updates to build GE0 geometry: that is, ME0 as part of GEM rather than a separate system.
Builds on the GE0 geometry xmls introduced in PR #29463 with the following:

changed m0_ to mg_ in muon numbering, this puts the ge0 layer in the "ring", I add 1 and this allows me to check for GE1/1 or GE2/1 (ring base number==0), or GE0 (ring base number =1-6 = GE0)
The GEMNumbering is updated to read as GE0 when mg_ring is not 0
I only updated the DDD section to be able to build GE0 (DD4HEP will be updated later)
I created a geometry based on D54 and placed the files in Geometry/GEMGeometry/test

@jshlee @bsunanda

PR validation:

I tested by writing a testGE0Geometry_cfg.py which loads the new geometry (GE0 as part of GEM geometry) and writing a script to compare the positions of the GE0 with the output of testME0Geometry_cfg.py; all corresponding eta partitions are in the same positions. I also checked that a 2026 upgrade workflow still runs correctly. Running with the new geometry requires changes upstream of geometry and will be made later.

if this PR is a backport please specify the original PR and why you need to backport that PR:

Before submitting your pull requests, make sure you followed this checklist:

verify that the PR is really intended for the chosen branch
verify that changes follow CMS Naming, Coding, And Style Rules
verify that the PR passes the basic test procedure suggested in the CMSSW PR instructions

Adds a new Geometry and a test file to check that the geometry can be built and to compare with ME0.

…26Geometry

cmsbuild · 2020-05-04T09:46:23Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-05-04T09:51:01Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29624/14993

This PR adds an extra 44KB to repository
Found files with invalid states:
- Geometry/CMSCommonData/python/cmsExtendedGeometry2026D59XML_cfi.py:
  - Added: 143c923
  - Deleted: 2f0bd8b
- Configuration/Geometry/python/GeometryExtended2026D59Reco_cff.py:
  - Added: 143c923
  - Deleted: 2f0bd8b
- Configuration/Geometry/python/GeometryExtended2026D59_cff.py:
  - Added: 143c923
  - Deleted: 2f0bd8b

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29624/14993/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29624/14993/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2020-05-04T10:12:30Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-05-04T10:17:13Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29624/14994

This PR adds an extra 44KB to repository
Found files with invalid states:
- Geometry/CMSCommonData/python/cmsExtendedGeometry2026D59XML_cfi.py:
  - Added: 143c923
  - Deleted: 2f0bd8b
- Configuration/Geometry/python/GeometryExtended2026D59Reco_cff.py:
  - Added: 143c923
  - Deleted: 2f0bd8b
- Configuration/Geometry/python/GeometryExtended2026D59_cff.py:
  - Added: 143c923
  - Deleted: 2f0bd8b

cmsbuild · 2020-05-04T10:18:37Z

A new Pull Request was created by @watson-ij (Ian J. Watson) for master.

It involves the following packages:

Geometry/GEMGeometry
Geometry/GEMGeometryBuilder
Geometry/MuonCommonData
Geometry/MuonNumbering

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @kpedro88 can you please review it and eventually sign? Thanks.
@jshlee, @ptcox, @dildick, @fabiocos, @slomeo this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

kpedro88 · 2020-05-04T15:33:44Z

Geometry/GEMGeometryBuilder/src/GEMGeometryBuilderFromDDD.cc

-          superChamber->add(
-              theGeometry.chamber(GEMDetId(detId.region(), detId.ring(), detId.station(), 2, detId.chamber(), 0)));
+          foundSuperChamber = true;
+          int nstation = (st == 0) ? 6 : 2;


magic numbers should be named constants

@watson-ij Please use these constants. https://github.com/cms-sw/cmssw/blob/master/DataFormats/MuonDetId/interface/GEMDetId.h#L30-L31. I also suggest to rename nstation to nlayers.

Suggested change

int nstation = (st == 0) ? 6 : 2;

int nlayers = (st == 0) ? GEMDetId::maxLayerId0 : GEMDetId::maxLayerId;

So, just to check @kpedro88 something like this would be okay, or I also need to introduce a named constant somewhere for st==0?

You could use GEMDetId::minStationId0

kpedro88 · 2020-05-04T15:39:45Z

please test

cmsbuild · 2020-05-04T15:40:05Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/5978/console Started: 2020/05/04 17:56

ianna · 2020-05-04T16:29:09Z

@slomeo - FYI

cmsbuild · 2020-05-11T05:45:14Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-05-11T05:51:25Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29624/15270

This PR adds an extra 48KB to repository
Found files with invalid states:
- Geometry/CMSCommonData/python/cmsExtendedGeometry2026D59XML_cfi.py:
  - Added: 143c923
  - Deleted: 2f0bd8b
- Configuration/Geometry/python/GeometryExtended2026D59Reco_cff.py:
  - Added: 143c923
  - Deleted: 2f0bd8b
- Configuration/Geometry/python/GeometryExtended2026D59_cff.py:
  - Added: 143c923
  - Deleted: 2f0bd8b
There are other open Pull requests which might conflict with changes you have proposed:
- File Geometry/GEMGeometryBuilder/src/GEMGeometryBuilderFromDDD.cc modified in PR(s): Run3-gem36 Use MuonGeometryConstants rather than MuonDDDConstants #29632
- File Geometry/MuonNumbering/src/GEMNumberingScheme.cc modified in PR(s): Run3-gem36 Use MuonGeometryConstants rather than MuonDDDConstants #29632

cmsbuild · 2020-05-11T05:51:50Z

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

kpedro88 · 2020-05-11T13:27:32Z

please test

cmsbuild · 2020-05-11T13:27:57Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6219/console Started: 2020/05/11 15:29

cmsbuild · 2020-05-11T14:39:45Z

+1
Tested at: 3acf2ff
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b6e6e5/6219/summary.html
CMSSW: CMSSW_11_1_X_2020-05-11-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-05-11T14:39:48Z

Comparison job queued.

cmsbuild · 2020-05-11T16:04:56Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2697527
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2697205
DQMHistoTests: Total skipped: 319
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

cvuosalo · 2020-05-11T16:14:46Z

+1

kpedro88 · 2020-05-11T16:25:21Z

+upgrade

cmsbuild · 2020-05-11T16:25:45Z

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. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2)

silviodonato · 2020-05-12T08:01:32Z

+1

watson-ij added 4 commits April 22, 2020 18:49

Update MuonNumbering and GeometryBuilder to build the GE0 in GEM

143c923

Adds a new Geometry and a test file to check that the geometry can be built and to compare with ME0.

Add script to check for differences between ME0 and GE0

e643f66

Merge branch 'master' into ge0-geometry-builder

5367ae7

Move "D59" into the GEMGeometry/test folder, remove changes to dict20…

2f0bd8b

…26Geometry

cmsbuild added this to the CMSSW_11_1_X milestone May 4, 2020

cmsbuild added code-checks-pending comparison-pending geometry-pending orp-pending pending-signatures tests-pending upgrade-pending labels May 4, 2020

cmsbuild added code-checks-rejected and removed code-checks-pending labels May 4, 2020

Code checks formatting patch

07df53a

cmsbuild added code-checks-pending and removed code-checks-rejected labels May 4, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels May 4, 2020

kpedro88 reviewed May 4, 2020

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels May 4, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels May 11, 2020

cmsbuild added tests-started and removed tests-pending labels May 11, 2020

cmsbuild added tests-approved and removed tests-started labels May 11, 2020

cmsbuild added comparison-available and removed comparison-pending labels May 11, 2020

cmsbuild added geometry-approved and removed geometry-pending labels May 11, 2020

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels May 11, 2020

cmsbuild added orp-approved and removed orp-pending labels May 12, 2020

cmsbuild merged commit 6715240 into cms-sw:master May 12, 2020

cmsbuild mentioned this pull request May 12, 2020

MTD geometry: porting of MTDNumberingBuilder to DD4hep #29647

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GE0 geometry builder #29624

GE0 geometry builder #29624

watson-ij commented May 4, 2020 •

edited

cmsbuild commented May 4, 2020

cmsbuild commented May 4, 2020

cmsbuild commented May 4, 2020

cmsbuild commented May 4, 2020

cmsbuild commented May 4, 2020

kpedro88 May 4, 2020

dildick May 4, 2020

watson-ij May 6, 2020

kpedro88 May 6, 2020

kpedro88 commented May 4, 2020

cmsbuild commented May 4, 2020 •

edited

ianna commented May 4, 2020

cmsbuild commented May 11, 2020

cmsbuild commented May 11, 2020

cmsbuild commented May 11, 2020

kpedro88 commented May 11, 2020

cmsbuild commented May 11, 2020 •

edited

cmsbuild commented May 11, 2020

cmsbuild commented May 11, 2020

cmsbuild commented May 11, 2020

cvuosalo commented May 11, 2020

kpedro88 commented May 11, 2020

cmsbuild commented May 11, 2020

silviodonato commented May 12, 2020

	int nstation = (st == 0) ? 6 : 2;
	int nlayers = (st == 0) ? GEMDetId::maxLayerId0 : GEMDetId::maxLayerId;

GE0 geometry builder #29624

GE0 geometry builder #29624

Conversation

watson-ij commented May 4, 2020 • edited

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented May 4, 2020

cmsbuild commented May 4, 2020

cmsbuild commented May 4, 2020

cmsbuild commented May 4, 2020

cmsbuild commented May 4, 2020

kpedro88 May 4, 2020

Choose a reason for hiding this comment

dildick May 4, 2020

Choose a reason for hiding this comment

watson-ij May 6, 2020

Choose a reason for hiding this comment

kpedro88 May 6, 2020

Choose a reason for hiding this comment

kpedro88 commented May 4, 2020

cmsbuild commented May 4, 2020 • edited

ianna commented May 4, 2020

cmsbuild commented May 11, 2020

cmsbuild commented May 11, 2020

cmsbuild commented May 11, 2020

kpedro88 commented May 11, 2020

cmsbuild commented May 11, 2020 • edited

cmsbuild commented May 11, 2020

cmsbuild commented May 11, 2020

cmsbuild commented May 11, 2020

cvuosalo commented May 11, 2020

kpedro88 commented May 11, 2020

cmsbuild commented May 11, 2020

silviodonato commented May 12, 2020

watson-ij commented May 4, 2020 •

edited

cmsbuild commented May 4, 2020 •

edited

cmsbuild commented May 11, 2020 •

edited