Conversation
* dodal devices motors script hereby has corrections applied to:
YXStage class now lists relevant motor axes as Y and X
* motor stage with 3 linear axes and 3 rotations is hereby labelled as
a six-axis ( rather than five ) stage
* inconsistencies in the layout of axes constants are thrashed out
(previously linear motors were all in a line, rotation axes isolated
perhaps it was a poetic metaphor ? )
* Grammar corrected when spelling out motors as a set of motors
rather than as one or more letters and a letter labelled motor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1936 +/- ##
=======================================
Coverage 99.05% 99.05%
=======================================
Files 314 314
Lines 11778 11780 +2
=======================================
+ Hits 11667 11669 +2
Misses 111 111 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
DominicOram
left a comment
There was a problem hiding this comment.
Thanks, couple of points. Only one blocking.
src/dodal/devices/motors.py
Outdated
| class Stage(StandardReadable, ABC): | ||
| """For these devices, the following co-ordinates are typical but not enforced: | ||
| - z is horizontal & parallel to the direction of beam travel | ||
| - z is horizontal & parallel to the direction of synchrotron particle travel (whence the x-rays direction) |
There was a problem hiding this comment.
Nit: Whence isn't a very commonly used word, particularly for people with English as a second language, I would just say (the x-rays direction)
There was a problem hiding this comment.
I didn't know that it is related to the direction of synchrotron particle - do you mean electrons? then it's quite unclear on which part of the ring - undulator? but then there are many mirrors that change photon light direction so final x-ray direction on the endstation is not necessarily same direction as electrons in undulator (also electrons in undulator have quite a jiggle). I would just leave x-ray beam direction.
P.S. I confirm I see "whence" probably second time in my life - had to google that.
There was a problem hiding this comment.
I also agree that beam direction makes more sense to me.
There was a problem hiding this comment.
I accept that ambiguous documentation is probably not the place to unpack the axis definition facility.
The basic point was: Particle accelerators usually take the accelerator physics definitions of z along the particle beam ( be they electrons, protons ... other particles are accelerated - eg pions etc ), then y is made to point up and x falls wherever it needs to to get the right handed cross product. In the case of a ring shaped accelerator this usually makes the x axis point away from the centre of the ring.
Therefore the axis direction convention for the facility comes from the particle beam not the x-ray beam.
( It just happens that the prevailing x ray direction followed the tangent from the electrons orbit )
There was a problem hiding this comment.
It's true whence / whither / wherefore are from the Germanic roots of English and used rarely.
But we didn't replace them with any shorter / more recognisable words and Romance / Germanic languages have a single word for the same concept - even if those languages have also followed the modern trend of replacing a single word with several. Whence = German woher, woraus Latin unde -> Italian donde but also da dove, da cui
I'm more inclined to agree with removing a flowery unusual word with a simpler more common alternative - when this is a single word.
Here I agreed to scrap whence and go with a sentence
src/dodal/devices/motors.py
Outdated
|
|
||
| class YZStage(Stage): | ||
| """Two-axis stage with an x and a z motor.""" | ||
| """Two-axis stage with an x motor and a y motor.""" |
There was a problem hiding this comment.
Should: Shouldn't this be Y and Z?
* dodal devices motors script hereby has corrections applied to: YZStage class now lists relevant motor axes as Y and Z * replaced whence with longer explanation
dodal devices motors script hereby has corrections applied to: YXStage class now lists relevant motor axes as Y and X
motor stage with 3 linear axes and 3 rotations is hereby labelled as a six-axis ( rather than five ) stage
inconsistencies in the layout of axes constants are thrashed out (previously linear motors were all in a line, rotation axes isolated perhaps it was a poetic metaphor ? )
Grammar corrected when spelling out motors as a set of motors rather than as one or more letters and a letter labelled motor
Replaces cross product symbol ( which was not rendering on github - nor in simple text editor )
with the phrase "cross product of y with z" ( in the description of the x axis direction definition )
Fixes #1935
Instructions to reviewer on how to test:
Checks for reviewer
dodal connect ${BEAMLINE}