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

Fix chimney hole #835

Closed
wants to merge 2 commits into from
Closed

Conversation

TaiSakuma
Copy link
Contributor

ChimHole_height was a bit too short for the solid subtractions to work properly. I made it long enough for the subtractions to succeed.

I think that the chimney hole was rotated to a wrong angle. I changed the angle such that the orientation of the hole makes more sense on the magnet.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @TaiSakuma (Tai Sakuma) for CMSSW_5_3_X.

Fix chimney hole

It involves the following packages:

Geometry/CMSCommonData

@Dr15Jones, @ianna, @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.
@smuzaffar you are the release manager for this.

@ianna
Copy link
Contributor

ianna commented Sep 16, 2013

This should be compared to an engineering drawing before it goes to a release. As far as I remember, there was a fix for that already.

@TaiSakuma
Copy link
Contributor Author

This is what I get without f914415

screen shot 2013-09-14 at 9 18 52 pm

This is with f914415

screen shot 2013-09-14 at 9 30 53 pm

@TaiSakuma TaiSakuma closed this Sep 16, 2013
@TaiSakuma TaiSakuma reopened this Sep 17, 2013
@ianna
Copy link
Contributor

ianna commented Sep 17, 2013

screen shot 2012-02-03 at 11 50 12 am

I think, none of them is correct. Please, see the drawing attached.

@ianna
Copy link
Contributor

ianna commented Sep 17, 2013

There was a fix from February 2012 to fix not only the rotation, but also a union shape subtracted from the volume which was not correct.

@ianna
Copy link
Contributor

ianna commented Sep 17, 2013

screen shot 2012-02-03 at 2 37 44 pm

@ianna
Copy link
Contributor

ianna commented Sep 17, 2013

@TaiSakuma please, test 57cd68d

@TaiSakuma
Copy link
Contributor Author

This is with 1e4c6fe and 57cd68d

screen shot 2013-09-17 at 7 58 22 am

The image looks the same as the screenshot from @ianna's but looks different from Fig 9.1.

Without 1e4c6fe, the height of CHIMNEY_HOLE_P and CHIMNEY_HOLE_N will be too short for the solid subtraction. It will be exactly the same as the thickness of the wall of the MGNT. And since MGNT has a cylindrical shape and its wall is curved, the solid to subtract needs to be somewhat taller than the thickness of the wall.

@ianna
Copy link
Contributor

ianna commented Sep 17, 2013

Thanks, Tai!

Yana

On Sep 17, 2013, at 3:09 PM, Tai Sakuma wrote:

This is with 1e4c6fe and 57cd68d

The image looks the same @ianna's post.

Without 1e4c6fe, the height of CHIMNEY_HOLE_P and CHIMNEY_HOLE_N will be too short for the solid subtraction. It will be exactly the same as the thickness of the wall of the MGNT. And since MGNT has a cylindrical shape and its wall is curved, the solid to subtract needs to be somewhat taller than the thickness of the wall.


Reply to this email directly or view it on GitHub.


Ianna Osborne
CERN, 6 2-013
CH-1211, Geneve 23, Suisse
+41 22 76 70968
ianna.osborne@cern.ch

@ianna
Copy link
Contributor

ianna commented Sep 17, 2013

hmm... the shape is correct, but it seems too big in comparison with the other one.

On Sep 17, 2013, at 3:09 PM, Tai Sakuma wrote:

This is with 1e4c6fe and 57cd68d

The image looks the same @ianna's post.

Without 1e4c6fe, the height of CHIMNEY_HOLE_P and CHIMNEY_HOLE_N will be too short for the solid subtraction. It will be exactly the same as the thickness of the wall of the MGNT. And since MGNT has a cylindrical shape and its wall is curved, the solid to subtract needs to be somewhat taller than the thickness of the wall.


Reply to this email directly or view it on GitHub.


Ianna Osborne
CERN, 6 2-013
CH-1211, Geneve 23, Suisse
+41 22 76 70968
ianna.osborne@cern.ch

@TaiSakuma
Copy link
Contributor Author

I agree. At least the chimney itself is much smaller from this photo
http://web.ethlife.ethz.ch/images/Cern-CMS-l.jpg

Also, the magnet seems to have another hole
http://cds.cern.ch/record/43056/files/magnet-2002-022_01.jpg?subformat=icon-1440

from a different angle with all cables.
https://cds.cern.ch/record/1274457/files/20071127_005.JPG?subformat=icon-1440
The chimney is near the top. This is much smaller. And then there is the bigger hole

@ianna
Copy link
Contributor

ianna commented Sep 17, 2013

-1
so, the inner opening in the release seems to have a correct width. I think, the other thing is a suspension point.

@TaiSakuma
Copy link
Contributor Author

I see.

I tried drawing MGNT_1 and CHIMNEY_HOLE_N.

screen shot 2013-09-17 at 10 37 44 am

dz of CHIMNEY_HOLE_N is the original value of [ChimHole_height], i.e. ([MGNT_rmax2]-[MGNT_rmin])/2.

I think CHIMNEY_HOLE_N is too long for an inner opening (and too short for a hole). Or if this is an inner opening, it needs to be bent to much the curvature of the MGNT.

But perhaps, this might not be important in CMSSW. Personally, I prefer this to be fixed because the solid subtraction fails in SketchUp and causes an error. But if this doesn't need to be fixed in CMSSW, I will probably just release some patch from SketchUpCMS/cmssw for SketchUp.

@ktf
Copy link
Contributor

ktf commented Sep 19, 2013

For the record. This will not make CMSSW_5_3_12_patch2 since it was not fully signed before the deadline for it.

@ktf
Copy link
Contributor

ktf commented Sep 19, 2013

After discussing with ianna, this is actually not going into 53X at all. Closing this.

@ktf ktf closed this Sep 19, 2013
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