Fix #1935 - corrections of discrepancies / inconsistencies in motors by CoePaul · Pull Request #1936 · DiamondLightSource/dodal

CoePaul · 2026-02-23T22:01:40Z

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:

Check corrections made to motors script
Confirm no new discrepancies added to documentation etc
Confirm CI doesn't fall over due to any spurious typos

Checks for reviewer

Would the PR title make sense to a scientist on a set of release notes
If a new device has been added does it follow the standards
If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

codecov · 2026-02-23T23:01:40Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.08%. Comparing base (36d3bf0) to head (8ba521d).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1936   +/-   ##
=======================================
  Coverage   99.08%   99.08%           
=======================================
  Files         314      314           
  Lines       11802    11804    +2     
=======================================
+ Hits        11694    11696    +2     
  Misses        108      108

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

DominicOram

Thanks, couple of points. Only one blocking.

DominicOram · 2026-02-24T08:53:50Z

src/dodal/devices/motors.py

 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)


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)

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.

I also agree that beam direction makes more sense to me.

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 )

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

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 )

Thanks for clarifying this. I was absolutely sure scientists care about aligning with x-ray beam in stage coordinate system, so while you are correct in strict definition of accelerator coordinate system maybe in reality on the beamlines it is often aligned with x-ray beam first (irrelevant whether it is aligned with electron momentum or not)?

Would tangent to the electron orbit be a easier to understand description than "horizontal & parallel to the direction of synchrotron particle travel"?

Would tangent to the electron orbit be a easier to understand description than "horizontal & parallel to the direction of synchrotron particle travel"?

I'm inclined to strong agree with You - we already have ( for the Y direction ) the highly pedantic language about being anti-parallel to gravity ( when "pointing up" would have been fine IMHO )

x-rays are particles too - it can get quite messy

scientists care about aligning with x-ray beam
True for a team on a given beamline
but the x,y,z are facility ( ie diamond light source ) definitions first -
which arose from accelerator physics conventions
( and dodal is - I assumed - trying to provide facility level definitions )

Within a beamline they are free to rename x,y,z --> u,v,w if they want to
( provided it only covers their own beamline )
But given that the rest of the documentation was unpacking the facility definitions of x,y,z I continued in that vein

DominicOram · 2026-02-24T08:54:09Z

src/dodal/devices/motors.py


 class YZStage(Stage):
-    """Two-axis stage with an x and a z motor."""
+    """Two-axis stage with an x motor and a y motor."""


Should: Shouldn't this be Y and Z?

* 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

* 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

* Amend documentation as per CR requests

CoePaul requested a review from a team as a code owner February 23, 2026 22:01

DominicOram requested changes Feb 24, 2026

View reviewed changes

CoePaul requested review from DominicOram and Villtord February 24, 2026 13:16

Villtord approved these changes Feb 25, 2026

View reviewed changes

CoePaul force-pushed the dodal1935 branch 2 times, most recently from 825f8d3 to 73abf30 Compare February 26, 2026 18:35

CoePaul added 3 commits February 27, 2026 17:00

Fix #1935 - further corrections as requested in code review

4618f10

* 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

Fix #1935 - code review requests round 2

8ba521d

* Amend documentation as per CR requests

CoePaul force-pushed the dodal1935 branch from 73abf30 to 8ba521d Compare February 27, 2026 17:00

Conversation

CoePaul commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Instructions to reviewer on how to test:

Checks for reviewer

Uh oh!

codecov bot commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DominicOram left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Villtord Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CoePaul commented Feb 23, 2026 •

edited

Loading

codecov bot commented Feb 23, 2026 •

edited

Loading

Villtord Feb 25, 2026 •

edited

Loading