Muon Cooler solenoid rotations #111
Conversation
|
Ok looks pretty straightforward. I'll add minor comments in review. Other issues
|
| true, | ||
| a + (sheet * dr) + dr / 2, | ||
| fullLengthZ, | ||
| tiltX, |
| G4bool strengthIsCurrent, | ||
| G4double sheetRadius, | ||
| G4double fullLength, | ||
| G4double tiltX, |
| I(0.0), | ||
| spatialLimit(std::min(1e-5*sheetRadius, 1e-5*fullLength)), | ||
| normalisation(1.0) , | ||
| rotateX(tiltX), |
There was a problem hiding this comment.
Try to be more consistent in variable names. Could be rotateX(rotateXIn)
There was a problem hiding this comment.
Agree - should be consistent.
|
|
||
| // Apply inverse rotations in reverse order (Z -> Y -> X) | ||
| // to transform from global frame to solenoid's local frame | ||
| localPosition.rotateZ(-rotateZ); |
There was a problem hiding this comment.
Why not formulate the transformation and just call the inverse method?
lnevay
left a comment
lnevay
left a comment
There was a problem hiding this comment.
Does it make sense to have a rotation about the local unit Z for a cylindrical symmetric field? So this just rotates the x,y offset?
Few comments for minor changes - thanks!
|
|
||
| **Rotations and Offsets** | ||
|
|
||
| Rotations follow the right-hand rule using axis-angle representation applied in XYZ order about the X, Y, and Z axes respectively. The reference frame assumes the solenoid is initially centered at the origin with its axis aligned along the z-direction. The position offsets then translate this solenoid center from the origin to an arbitrary location in the global coordinate system of the cooling channel. |
There was a problem hiding this comment.
"offsets then translate(s)".. just translates. I think not in the global frame but in the coordinate frame of the cooling channel element.
| I(0.0), | ||
| spatialLimit(std::min(1e-5*sheetRadius, 1e-5*fullLength)), | ||
| normalisation(1.0) , | ||
| rotateX(tiltX), |
There was a problem hiding this comment.
Agree - should be consistent.
| // Apply inverse rotations in reverse order (Z -> Y -> X) | ||
| // to transform from global frame to solenoid's local frame | ||
| localPosition.rotateZ(-rotateZ); | ||
| localPosition.rotateY(-rotateY); |
There was a problem hiding this comment.
Like Stewart mentioned, I would perhaps use a G4RotationMatrix. You can prepare it in the constructor and the inverse as well. Then you can just multiply by the appropriate one each time.
| globalField.rotateY(rotateY); | ||
| globalField.rotateZ(rotateZ); | ||
|
|
||
| return globalField; |
There was a problem hiding this comment.
Please can you change this to some other name. The "globalField" is used in BDSIM to mean in the global frame, whereas this is still in the local frame of the element. You usually use one mega-cooling channel but conceptually it's 'local' as the global transform for the beamline is done in a wrapper class. How about unrotatedField and localField.
3 participants
No description provided.