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

GEM21 geometry #1809

Merged
merged 15 commits into from Dec 18, 2013
Merged

GEM21 geometry #1809

merged 15 commits into from Dec 18, 2013

Conversation

ianna
Copy link
Contributor

@ianna ianna commented Dec 13, 2013

  • GE21 short chambers are fully implemented, readout board is still missing
  • GE21 long chambers need sensitive volumes and readout board

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ianna for CMSSW_7_0_X.

GEM21 geometry

It involves the following packages:

Geometry/MuonCommonData
Geometry/CMSCommonData

@civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @ktf can you please review it and eventually sign? Thanks.
@ghellwig this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@ktf you are the release manager for this.

@cmsbuild
Copy link
Contributor

'Geometry/MuonCommonData/data/v2/rpcf.xml',
'Geometry/MuonCommonData/data/v4/gemf.xml',
'Geometry/MuonCommonData/data/v6/gemf.xml',
'Geometry/MuonCommonData/data/gem21.xml',
Copy link
Contributor

Choose a reason for hiding this comment

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

typo? Shouldn't this be data/v6/gem21.xml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, I'll fix it on Monday.

Copy link
Contributor

Choose a reason for hiding this comment

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

From now on I'm "Eagle Eye Giulio"...

@ianna ianna closed this Dec 16, 2013
@ianna ianna reopened this Dec 16, 2013
@cmsbuild
Copy link
Contributor

Pull request #1809 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @ktf can you please check and sign again.

@ianna
Copy link
Contributor Author

ianna commented Dec 16, 2013

@dildick - please, take this branch. Thanks.

<rSolid name="RED4"/>
<rMaterial name="materials:M_RPC_Gas"/>
</LogicalPart>
</LogicalPartSection>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still not convinced why this RPC part should be present in the GEM geometry definition. Can we leave it out in all prototypes without harm?

@cmsbuild
Copy link
Contributor

Pull request #1809 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @ktf can you please check and sign again.

@ianna
Copy link
Contributor Author

ianna commented Dec 17, 2013

@dildick - please, check. Thanks!

@cmsbuild
Copy link
Contributor

Pull request #1809 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @ktf can you please check and sign again.

@ianna
Copy link
Contributor Author

ianna commented Dec 17, 2013

@dildick - Please, try it now. Thanks.

@dildick
Copy link
Contributor

dildick commented Dec 17, 2013

@ianna The GE2/1 eta partitions are now found by the geometry builder, but they are assigned the same detIds as their GE1/1 counterparts. (Station number is put to 1) A minor change needs to be done in the numbering XML file. You can use this recipe to debug it. (It already contains all your additions)

cmsrel CMSSW_6_2_X_SLHC_2013-12-09-1400
cd CMSSW_6_2_X_SLHC_2013-12-09-1400/src
cmsenv
git cms-merge-topic --unsafe dildick:ge21-geometry-test
git cms-addpkg Geometry/GEMGeometry
git checkdeps -a
scram b -j 9

cmsRun Geometry/GEMGeometry/test/testGEMGeometry_cfg.py

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Pull request #1809 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @ktf can you please check and sign again.

@ianna
Copy link
Contributor Author

ianna commented Dec 18, 2013

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next IBs unless changes (tests are also fine). @ktf can you please take care of it?

ktf added a commit that referenced this pull request Dec 18, 2013
Geometry updates -- GEM21 geometry
@ktf ktf merged commit e1852f6 into cms-sw:CMSSW_7_0_X Dec 18, 2013
@dildick
Copy link
Contributor

dildick commented Dec 18, 2013

@ianna

I ran the geometry tester on this PR. Can you fix the XML file? Thanks.

[lxplus443] /afs/cern.ch/user/d/dildick/work/GEM/testForYana/CMSSW_7_0_X_2013-12-18-0200/src/Geometry/GEMGeometry/test > !cmsR
cmsRun testGEMGeometry_cfg.py
======================== Opening output file
Begin processing the 1st record. Run 1, Event 1, LumiSection 1 at 18-Dec-2013 13:10:39.386 CET
expr: gem21___dyGap4
------^
Evaluator : unknown variable
----- Begin Fatal Exception 18-Dec-2013 13:10:42 CET-----------------------
An exception of category 'DDException' occurred while
[0] Processing run: 1 lumi: 1 event: 1
[1] Running path 'p'
[2] Calling event method for module GEMGeometryAnalyzer/'test2'
[3] Using EventSetup component GEMGeometryESModule/'' to make data GEMGeometry/'' in record MuonGeometryRecord
[4] Using EventSetup component XMLIdealGeometryESSource/'' to make data DDCompactView/'' in record IdealGeometryRecord
Exception Message:
ClhepEvaluator ERROR: can't evaluate: [dyGap4]!
nmspace=gem21
varname=gem21___dyGap4
exp=[dyGap4]
at=
----- End Fatal Exception -------------------------------------------------

@ianna
Copy link
Contributor Author

ianna commented Dec 18, 2013

@dildick - ok, I've added the constant, but I'll need to resubmit it in a different branch.

@cmsbuild
Copy link
Contributor

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

4 participants