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

Updated CASTOR geometry to support to separated halves #2722

Merged

Conversation

cbaus
Copy link

@cbaus cbaus commented Mar 5, 2014

Why we need the change:
The updated geometry can decribe a gap in the 2 (physical) detector halves. Previously castor was only 1 geant volume. We have reason to believe that due to the magnetic field castor is separated by ~1.5mm. This commit keeps the nominal position in castor.xml for the moment.

What is changed:
This includes changes in the sector to volume numbering scheme in order match the original numbering (CastorNumberingScheme.xx)
The geant volumes are duplicated and each half contains 8 sectors. (castor.xml)
Also added the possibility for a rotation or tilt as we call it. (castor.xml)

Validation:
The geometry has been validated extensively in CMSSW_4_4 (we will prepare some slides) and I just ran for 7_1_X 500k pion events (each old and new) and compared nominal to nominal which looked good in the statistical uncertainty.

Future:
We already obtained a position from different measurements in 2013. This has to be integrated.
One remark. The position can in principle change for each run. It may be needed to store x and y for each half in the database. I will discuss this in the next CASTOR meeting. The changes here will still be required.

Example for validation plot:
Comparison for sector numbering

… changes in the sector to volume numbering scheme in order match the original numbering.
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2014

A new Pull Request was created by @cbaus for CMSSW_7_1_X.

Updated CASTOR geometry to support to separated halves

It involves the following packages:

Geometry/ForwardCommonData
SimG4CMS/Forward

@civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @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.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cbaus
Copy link
Author

cbaus commented Mar 5, 2014

@iik1997 @hvanhaev
CASTOR team, please follow. Thx.

@ianna
Copy link
Contributor

ianna commented Mar 5, 2014

@cbaus - there are a few overlaps (9 extrusions) in CASTOR geometry. Could these be fixed, please?
Thanks. Yana

screen shot 2014-03-05 at 09 30 40

@cbaus
Copy link
Author

cbaus commented Mar 5, 2014

@ianna
This is no excuse but I want to state that the old gemetry had these overlaps as well. I would like to take care of this problem but I do not know how to fix them. Do you have a tutorial TWiki page handy for this? I know how to display them but I don't understand how they are created. Is this not a numerical problem since all these small sections are simply rotated to 22.5°, 45°, .. ?
Why do some have overlaps and others do not?

Sorry to bother you with this and thank you for any hints.
Colin

@civanch
Copy link
Contributor

civanch commented Mar 5, 2014

@cbaus,
Small comments: Geant4 may run silently even if a geometry has some problems, so it may be there for a long time; if the logical volume is placed many times (different rotations) then one problem is repeated many times, so I would look in sizes and positions of volumes identified in ianna report.

@ianna
Copy link
Contributor

ianna commented Mar 7, 2014

@civanch and @cbaus - yes, the overlap problem was there since a long time - unfortunately, it has not been fixed. The tool reports one overlap per one shape copy. It means, that all the other copies have the same overlap. The best way to fix it is to compare overlapping shape descriptions in xml. More information on how to use the tool is here: https://twiki.cern.ch/twiki/bin/view/CMSPublic/WorkBookFireworksGeometry

@nclopezo nclopezo modified the milestones: CMSSW_7_1_0_pre5, CMSSW_7_1_0_pre4 Mar 10, 2014
@ianna
Copy link
Contributor

ianna commented Mar 18, 2014

+1

@cbaus - I hope, the overlap fixes come soon

@cmsbuild
Copy link
Contributor

@cbaus
Copy link
Author

cbaus commented Mar 19, 2014

@ianna @civanch Thank you both for your comments.

Over the past 2 weeks there have been a lot of discussions about the overlaps and extrusions in the CASTOR meetings. As of today we believe to understand their origin and and might also have a solution how to fix them. We will work on this and let you know as soon as it's done. Taking into account internal validation, however, we cannot promise to finish by end of March.

Since this particular pull request of dividing CASTOR in 2 halves does no changes to the overlaps, we propose to accept the request for now and we will add a new one for the overlaps/extrusions. Is this alright with you?

@civanch
Copy link
Contributor

civanch commented Mar 19, 2014

+1
OK for me

@cmsbuild
Copy link
Contributor

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

ktf added a commit that referenced this pull request Mar 19, 2014
…-halves

Sim -- Updated CASTOR geometry to support to separated halves
@ktf ktf merged commit d3a1fe6 into cms-sw:CMSSW_7_1_X Mar 19, 2014
@civanch
Copy link
Contributor

civanch commented Apr 23, 2014

@cbaus, there is a problem likely connected with this PR see discussion 👍 https://hypernews.cern.ch/HyperNews/CMS/get/simDevelopment/1641.html

There are too many warnings about wrong detId.
Can you, please,comment?

ggovi pushed a commit to ggovi/cmssw that referenced this pull request Jan 11, 2017
update build rules tag to V05-05-19
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