Repository navigation
Standard way to define device once that is shared between multiple end statations #1530
Description
It is common for branched beamlines to have shared components, e.g PGM is used by I06 and I06-1.
In dodal, we don't seem to have a standard way of doing this as of yet. At the moment, we have dodal/beamlines/i05.py, dodal/beamlines/i05_1.py and dodal/beamlines/beamline_specific_util/i05_shared.py. However, i05.py and i05_1.py both redefine the pgm which defeats the purpose were trying to achieve.
from dodal.beamline_specific_utils.i05_shared import pgm as i05_pgm
@device_factory()
def pgm() -> PGM:
return i05_pgm()In other situations, we have I09 defined DCM in two different places when it's the same device i09.py and i09_1.py.
This should be defined once in it's own module. This seems to be what I19 have done with i19_1.py, i19_2.py and i19_optics.py, though I think this is slightly different use case as this is a way for access control and has its own BlueAPI service. However, I believe all beamlines, if has shared endstations, should have devices defined once in a shared module. This way, we get BlueAPI to pull devices from beamline iXX and iXX_shared to be into a single environment e.g
worker:
env:
sources:
- kind: planFunctions
module: sm_bluesky.beamlines.i05.plans
- kind: planFunctions
module: sm_bluesky.common.plan_stubs
- kind: planFunctions
module: sm_bluesky.common.plans
- kind: planFunctions
module: dodal.plans
- kind: dodal
module: "dodal.beamlines.i05"
- kind: dodal
module: "dodal.beamlines.i05_shared"
I think we can also go further and maybe define a dodal/beamlines/core.py as well because almost every beamline defines Synchrotron device, we can remove this duplication.
Finally, as this makes beamlines more modular and we want to test using dodal connect iXX for a whole beamline, I suggest the following change be made to dodal connect:
We define some config in dodal e.g
_BEAMLINE_SHARED = {
"i09": ["i09", "i09_1_shared", "i09_2_shared"]
...
}This config defines the whole beamline so when running dodal connect i09, we get:
(venv) root ➜ /workspaces/dodal (fix_test_motor_limits_with_new_ophyd_async ✗)
$ dodal connect i09 -s
====================================================================================================
Attempting connection to i09 (using dodal.beamlines.i09)
2 devices connected (sim mode):
analyser_driver
synchrotron
====================================================================================================
Attempting connection to i09_1_shared (using dodal.beamlines.i09_1_shared)
1 devices connected (sim mode):
dcm
====================================================================================================
Attempting connection to i09_2_shared (using dodal.beamlines.i09_2_shared)
1 devices connected (sim mode):
pgm
We can also add a flag to test the individual beamline only
(venv) root ➜ /workspaces/dodal (fix_test_motor_limits_with_new_ophyd_async ✗)
$ dodal connect i09 -s -i
====================================================================================================
Attempting connection to i09 (using dodal.beamlines.i09)
2 devices connected (sim mode):
analyser_driver
synchrotron
Acceptance Criteria
- Documentation added on standard for shared modules
- Beamlines conform to standard.
- Dodal connect command adjusted
Activity
DominicOram commentedon Sep 15, 2025
The i19 solution, documented at https://diamondlightsource.github.io/dodal/main/explanations/decisions/0006-devices-shared-between-endstations.html is partly over-engineered because there is a consistent issue where
19-1will be running a test and accidentally change the optics device that is currently in use oni19-2. In GDA there was no real way to reliably stop this but we have an opportunity to do so with Bluesky. I would suggest before doing this we confirm that the scientists of other branch beamlines see no value in this for their branches. It also appears from #1490 that @DiamondJoseph has some opinions on how this is done so it would be good to get his take.callumforrester commentedon Sep 15, 2025
Happy to establish this as a starting point, IMO we haven't yet thought enough about access control to propose a solution yet, so leave that for later.
callumforrester commentedon Sep 15, 2025
WRT a core device module, here's some 2 cents from someone who's leaving and has no skin in the game:
I don't like code-as-configuration, I like nicely constrained configuration that isn't turing-complete because you're far less likely to encounter nasty surprises that are hard to debug. I would eventually want all device instantiation to be controlled by YAML/TOML files (or a database) and no user-written Python code at all. It may be useful to think of a solution that doesn't design this out. I don't think this does but I'd appreciate someone else thinking about it.
I have previously suggested a tagging system, so we can tag devices as "facility-wide", "i05-optics-hutch" etc. and then get blueapi to scan and find devices that match a list of tags. Might also be relevant here.
oliwenmandiamond commentedon Sep 15, 2025
I agree with having beamlines created via configuration files like YAML rather than python code configuration files, though not sure how I feel with tagging system
fajinyuan commentedon Sep 15, 2025
Let me provide some description on current implementation and requirements on beamlines in MMG and SSG groups. In term of how to handle shared devices access different beamlines require different things depending on beamline desgin, configuration and operations. For MMG group, there are 2 beamlines - i06 and i10 with multiple end stations and branches. Both beamlines have a switch mirror that defines which end station or branch having X-ray beam for user operations at any specific time. This is controlled by beamline PBS. Its position triggers an upadte on a beamline specific PV which to tell us which end station or branch has the X-ray beam at that time.
i06 sharded devices are curently configured to intercept control/set access to check if the requesting branch should have access to move the device. It only allows the move request to go through to EPICS if it matches the value of this PV, and tell users if they don't have access to the device. I06 also requires beamline staffs to be excepted from this - i.e. if the user is a beamline scientist he or she can control the device from either branch or endstation. This is implemented in GDA via Spring AOP support currently.
However for i10, this is not implemented because i10 does not have users from differen branches operate in different cabin, so beamline scientists believe this will physically prevent 2 different set of users operate on the same shared device at the same time as they are physically in the same tiny room.
In SSG group,
i05 users operate shared devices from the same cabin, so the situation is very much like i10. I am not sure if i05 has or has not implemented AOP or not.
i07's two experiment hatches are arranged as EH2 before EH1. so at any one time only one beamline can have X-ray beam. While EH1 has beam, EH2 end station diffractometer should not be able to move because there is a beam vacuum tube flying through it. However EH2 might has users to do setup process without diffractometer access (I am not quiet clear at the moment how i07 does
this). Should EH2 has beam, users in EH1 should have access to EH1 devices with read only access to any shared devices.
i09's use cases are more complex then others. i09 has 2 physical X-ray - one covers software X-ray energy range and the other convers hard X-ray energy range. For cabin i09 users who might use both beams or only one of them at any one time. The beamline is designed to put both X-ray beams onto the same sample spot alternatively in a single experiment data collection using fast shutters. When i09 uses Hard X-ray there will be no users in i09-1 cabin before i09 cabin, when i09 uses soft X-ray there will be no users in i09-2 cabin before i09 cabin. However, should i09-1 has users using Hard X-ray, both i09 and i09-2 could have users using software X-ray beam but not at the same time. the same happens should i09-2 has users using software X-ray both i09 and i09-1 might have users to use Hard X-ray but not at the same time. Hard X-ray and Soft X-ray are generated by different insertion devices so there are no shared device bewteen the 2 physical beams. The sharing is between i09 and i09-1 for Hard X-ray, and i09 and i09-2 for soft X-ray. At the moment there are no access interception or checks implemented in GDA as beamline staffs decided their situations are complex, so they would physically enforce the rule in their user operations to avoid any potential of different user groups operating from different cabins on the same shared device.
I hope the above decriptions give everyone some ideas of the requirements for the new system to consider in their desgin for a generic solution to support these diverse use stories. Should you need any further information please let me know or communicate with our beamline scientists directly but let us know the decision you agreed with them so we can provide beamline with knowledge-based support more effectively. Thank you.
fajinyuan commentedon Sep 15, 2025
I absolutely support @callumforrester suggestion of using YMAL or TOML to specify beamline-specific instance list which takes beamline configuration, customisation, or specialisation out of python script. This if we succeeded will also give us a physical separation between codes/classes and objects beamlines use and would further encourage code sharing and generic coding across beamlines.
we should mention this as a mater of priority in our priority meetings as this will have great impact in support teams' development at the moment.
olliesilvester commentedon Sep 19, 2025
Just so we have it written down: In yesterday's athena drop-in session it was generally agreed that it's better to have duplication for, eg
i05.pyandi05_1.pythan it would be to create something likei05_shared.py.We should make sure everyone is on the same page with that (rather than just those who attended yesterdays session) and then also document that decision
fajinyuan commentedon Sep 19, 2025
Thanks @olliesilvester for adding this comment. I think we still need to discuss this matter in next DAQ team meeting. As one of the support team for MMG and SSG, I am strongly disagree with the recommendation you recorded in your comment. Duplication of instances for shared devices for beamlines having multiple end stations and branches will be a nightmere for long term supports as changes to one of end station/branch will not appear in others. We had these issues in current systems, we learnt our lessons, and we would like to not repeat these problems in the new system.
By default new system should always be better that old system - that is the critical point to carry beamline scientists with us on this journey of migration data acquisition software. Otherwise we will face an unwinable battle to fight.
fajinyuan commentedon Sep 19, 2025
I support @callumforrester 's suggestion above to move beamline-specific instance declaration and dependency into YAML or TOML file (like a replacement of Spring beans in GDA). This way we will have clear separation in beamline customisation or specialisation from the core source codes they are using, and we will not need to import objects in python script without using them (as RUFF check remove them automatically currently if they are presence) so we are not repeat the mistake we did in GDA localStation.py for each beamline.
Villtord commentedon Sep 19, 2025
I agree with Fajin, but in the meantime while we discuss - does it mean I remove already existing I05_shared.py from beamline_specific_utils folder?
DiamondJoseph commentedon Sep 22, 2025
Are there notes from the discussion? Sounds like we may need to have a wider discussion with Fajin involved.
I support initially having
synchrotrondefined for each beamline module even if it is duplication- it allows configuring it in sim mode (e.g. pxx), or omitting it (e.g. non-beamline instruments) and also serves as a good initial state, a "Hello, World!" statement of intent that a. this instrument is using dodal to track its devices, b. here is an example of what to do to add more, without leaving the file. The instrument is then all defined in a single place, and can be seen all at once.This can be extracted into e.g.
ixx_sharedto prevent duplication later and ensure shared components are in a consistent state, once the instrument has been populated more?fajinyuan commentedon Sep 22, 2025
Thank you @DiamondJoseph for your update. I think we do need to discuss in the big teams on this issue. I need to understand the evidence and reasons for the suggestion of duplcation of shared devices among end-stations or branches of a single beamline. I have given use cases from our set of beamlines above to give everyone some idea on the different spread of beamline's requirements. I must emphasis these only concerning shared device hardwares within a single beamline, not across beamlines. There might be or will be duplications across beamlines as different beamlines might or will require different specialisation.
We also need to discuss if there is a better way to specify or deliver beamline customisation or specialisation without duplication in codes.
callumforrester commentedon Sep 23, 2025
@fajinyuan I think we should be clear that we're talking about duplicate configuration, not duplicate code. I'm much less concerned about duplicate configuration, since configuration is supposed to be per-beamline anyway and the giant configuration hierarchies of GDA are just hard to follow/read and I would prefer (in order of preference):
So basically I think we should aim for a world where all of the configuration for a beamline is <100 lines and manageable by a human being.
fajinyuan commentedon Sep 23, 2025
we would prefer no duplication even in configuration as well, and also find a way to write configuration not in python scripts.
GDA bean configuration is big XML file, but there are no duplication within a single beamline setup. I hope with YAML the configuration will be less verbose than XML.
We did have bad experience in GDA when bean configuration was duplicated which caused inconsistency between branches in current system. Any duplication will have long term impact on support and maintanence as well.
oliwenmandiamond commentedon Sep 23, 2025
The issue at the moment is that configuration at the moment is still Python code. The second is that duplicate configuration should still be avoided. Supporting beamlines where your updating configuration of a device that is used by both branches and it doesn't apply to both risks them becoming out of sync and causing issues further down the line. For example, it is easy for someone to forget a device needs to be updated in multiple places rather than just the one.
DiamondJoseph commentedon Sep 24, 2025
I've managed to get #1490, where the
setmethod of a Movable can be locked behind an unlock predicate (e.g. can only be moved by the currently active endstation), to the point where it is ready for review finally. Highlighting it here for input and comment.callumforrester commentedon Sep 26, 2025
@fajinyuan sorry, when you say inconsistency between branches, do you mean version control branches or physical beamline branch lines?
If the former: Stop running the beamline on a branch for 6+ months, that's the actual bad practice going on in GDA-land. If the latter: Are you sure there's no legitimate reason for two branch lines to have differing configuration for the same underlying optics?
fajinyuan commentedon Sep 26, 2025
Mark-Booth commentedon Oct 2, 2025
I can understand why we didn't want to be side-tracked by this yesterday, but it is important to note that not all shared devices are shared on the same beamline. Some are shared between beamlines. Also the way they are shared can be different too, and it's important than when we make these decisions about intra-beamline shared devices, we don't make it more difficult to share devices intra-beanline.
Some Moveable Equipment (Diamond Launcher > Beamlines > Moveable Equipment) have their own PVs, which stay the same when they're moved between beamlines (IIRC).
Others however, run beamline specific IOCs on each beamline network, so their PVs change when moved to another beamline. An example of this is is i22's pressure cell, which is often used by i15.
AT one time, this kind of equipment was the bane of DAQ support staffs lives, with us having manually hack our configs every time something changed (often two configs had to be edited and two GDA servers had to be restarted for each move). More recently we've implemented profiles to allow staff to start up the GDA server with or without this moveable equipment, but as far as I can see, we don't have anything like this capability in dodal.
DominicOram commentedon Oct 3, 2025
For movable devices. We're currently at the start of a project in MX where we have a device that will be moving between DLS and XFEL. For this we're thinking of having a server sat next to the device that runs the ophyd-devices and some plans. I haven't got strong opinions on how we do this in general except that it's worth remembering we're moving away from a monolith and towards services. We should make sure we think about if we can leverage that
fajinyuan commentedon Oct 3, 2025
Thanks for your comments and descriptions of use cases for mobile devices - that is devices which its physical location moves @DominicOram and @Mark-Booth. I agree that they share some characteristics of shared devices location-fixed for beamlines. Could I kindly ask you create a new ticket for 'mobile devices' and I would like not to stratch this ticket too far and it should stay focus on 'shared devices in beamlines which have multiple end stations' so we don't loss the track of the issue.
There might a need to have a parent ticket to host the common requirements and use cases between shared devices and mobile devices if you feel approperiate. Appology if you feel I talk about rubish here, but do let me know if you feel my suggestion here is not correct.