First pass at allowing for cached devices to be used as args to enabl… by jwlodek · Pull Request #356 · pcdshub/happi

jwlodek · 2025-02-12T22:52:58Z

…e ophyd-async support

Description

Adds OphydAsyncItem class that allows for filtering for ophyd-async devices only since they require an additional step to establish a connection to the control system.
Adds the ability to use cached devices as arguments and kwargs, which allows for instantiating some of the more complex ophyd async devices.
Adds example database that instantiates an ophyd async detector + required provider objects.
Adds minimal example script to outline how ophyd async devices can be loaded.

Motivation and Context

Allows for seamless utilization of happi w/ ophyd-async devices.

How Has This Been Tested?

TODO: Unit tests, ensure existing test suite passes. Marking as draft for review & until I have a chance to complete these.

I did test with a real camera:

(venv) [jwlodek@xf31id1-ioc1 examples]$ ipython
Python 3.12.8 (main, Dec  9 2024, 15:25:01) [GCC 8.5.0 20210514 (Red Hat 8.5.0-22)]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.32.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: %run load_ophyd_async_devices.py
Initializing bluesky RunEngine...
Instantiating all helper objects...
Loading uuid_fp [ophyd_async.core.UUIDFilenameProvider] ... SUCCESS!
Loading ymd_pp [ophyd_async.core.YMDPathProvider] ... SUCCESS!
Initializing Ophyd Async devices...
Loading manta_cam1 [ophyd_async.epics.advimba.VimbaDetector] ... SUCCESS!
Loading ophyd_async_sim_motor_1 [ophyd_async.sim.SimMotor] ... SUCCESS!
Loading ophyd_async_sim_motor_2 [ophyd_async.sim.SimMotor] ... SUCCESS!
Done.

In [2]: RE(count([manta_cam1], num=5))
Out[2]: ('1f910933-9d0e-4962-96c9-7f6474b834be',)

In [3]: !ls /tmp/manta_cam1/2025/02/12/
6f6db019-7869-43bb-a1fe-4ec4dae9e42c.h5  ad9f9d4b-a8fc-42e6-96aa-f7c7a0523b7b.h5

In [4]: !h5dump /tmp/manta_cam1/2025/02/12/ad9f9d4b-a8fc-42e6-96aa-f7c7a0523b7b.h5
HDF5 "/tmp/manta_cam1/2025/02/12/ad9f9d4b-a8fc-42e6-96aa-f7c7a0523b7b.h5" {
GROUP "/" {
   GROUP "entry" {
      ATTRIBUTE "NX_class" {
         DATATYPE  H5T_STRING {
            STRSIZE 8;
            STRPAD H5T_STR_NULLTERM;
            CSET H5T_CSET_ASCII;
            CTYPE H5T_C_S1;
         }
         DATASPACE  SCALAR
         DATA {
         (0): "NXentry"
         }
      }
      GROUP "data" {
         ATTRIBUTE "NX_class" {
            DATATYPE  H5T_STRING {
               STRSIZE 7;
               STRPAD H5T_STR_NULLTERM;
               CSET H5T_CSET_ASCII;
               CTYPE H5T_C_S1;
            }
            DATASPACE  SCALAR
            DATA {
            (0): "NXdata"
            }
         }
         DATASET "data" {
            DATATYPE  H5T_STD_U8LE
            DATASPACE  SIMPLE { ( 5, 544, 728 ) / ( H5S_UNLIMITED, 544, 728 ) }
...

Where Has This Been Documented?

TODO

Pre-merge checklist

Code works interactively
Code contains descriptive docstrings, including context and API
New/changed functions and methods are covered in the test suite where possible
Test suite passes locally
Test suite passes on GitHub Actions
Ran docs/pre-release-notes.sh and created a pre-release documentation page
Pre-release docs include context, functional descriptions, and contributors as appropriate

…e ophyd-async support

tangkong

Left a couple of thoughts, on the whole I think this is an interesting idea. My main concern is the way this becomes dependent on the order in which we load devices, which I don't think happi makes any guarantees about.

I wonder if an alternate approach might be:

attempt to load all devices
collect any items that have args that can't be filled by metadata and defer their instantiation
After the first pass, attempt to fill and load the previously deferred objects
repeat until done or the deferred list doesn't change in size?

What I suggest is a bit more involved than what you've put forward here, but it might allow us to not have to call load_devices more than once

tangkong · 2025-02-13T00:37:25Z

+
+
+class OphydAsyncItem(OphydItem):
+    ...


If this is being subclassed with no changes, is it really necessary at all?

I agree with Robert. Perhaps it's more consistent and thorough to do a simple isinstance check on the created device for whether it's from ophyd-classic, ophyd-async, or neither, rather than trusting ourselves to use containers to mark them.

My other thought was adding an optional additional metadata key to the ophyd item class called async_device that's a boolean. I'd prefer to be able to determine the device type prior to loading so I can treat them slightly differently. How does that sound as opposed to a seperate class?

I think a simple boolean key is a reasonable approach, instructing the loader to try instantiate the object after a first pass is completed

While your primary use case is ophyd async, I might shy away from naming any of this with ophyd-async specific terminology. happi can be used to load any arbitrary python object, and that's a generality I wouldn't want to lose

tangkong · 2025-02-13T01:09:56Z

+    pprint=True,
+    include_load_time=True,
+    post_load=lambda device: RE(ensure_connected(device)),
+    namespace=init_ns,


When I first read this I thought that the intent was to have happi reference this init_ns when trying to fill devices, but this isn't the case. As written the reason this works is because of the execution order of the happi.loader.load_devices calls.

Just to be sure, was this your intent?

My original idea was to pass around a reference to init_ns, but this got a bit messy, and functionally acted the same as just using the cache - in both cases it working would be order dependant, so I figured for this initial proof of concept just using the cache would be easier with fewer code changes.

ZLLentz · 2025-02-14T01:15:35Z

+        "name": "uuid_fp",
+        "type": "HappiItem"
+    },
+    "ymd_path_provider": {


Small aside, I don't remember happi being able to generate json files where the top-level keys differ from the name field. Am I remembering wrong?

To be honest these were mostly copy pasted and hand edited rather than generated via the client, so very likely this wouldn't be able to be generated in-code.

ZLLentz · 2025-02-14T01:21:28Z

+    attr_name = info.pop()
+
+    # If the templated variable is in the cache, use it directly.
+    if attr_name in cache:


I think the order-dependency of this is a bit antithetical to the other design elements in happi and puts some burden on the user.

I should be able to request manta_cam1 and happi should figure out that it also has to instantiate ymd_pp to pass in as an argument, for example (and then simply return manta_cam1 since I didn't ask for a direct reference to ymd_pp).

To this end, maybe there needs to be special syntax to more cleanly separate other-happi-device args from basic args. Then rather than always checking the cache for all attributes, happi parses the device marker and knows that it either needs to pull the cached device or make the requested device now.

This increases complexity a bit, particularly in how this interacts with post_load handlers. Maybe that's enough of a reason to defer it until later after more thought/discussion.

I tend to agree. The order dependency is not super nice, it would be better if there could be a way for happi to automatically generate a load order as needed. As you say though this gets somewhat more complicated then - presumably we'd need a client available during loading so we could find the dependant devices?

That's a good point, we'd need to be able to pass the client through the whole from_container/load_device chain, preferably optionally

ZLLentz · 2025-02-14T01:22:59Z

I think this is a solid idea and I appreciate you starting with a simple and effective approach

…ey instead

First pass at allowing for cached devices to be used as args to enabl…

41c27e5

…e ophyd-async support

tangkong requested review from ZLLentz and tangkong February 13, 2025 00:52

tangkong reviewed Feb 13, 2025

View reviewed changes

ZLLentz reviewed Feb 14, 2025

View reviewed changes

jwlodek added 2 commits February 14, 2025 09:57

Remove seperate ophyd async class, make a boolean optional metadata k…

3431e2e

…ey instead

Re-order checks to make sure to use item attr before cache entry

40c1d9f

Conversation

jwlodek commented Feb 12, 2025

Description

Motivation and Context

How Has This Been Tested?

Where Has This Been Documented?

Pre-merge checklist

Uh oh!

tangkong 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZLLentz Feb 14, 2025 • 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!

ZLLentz commented Feb 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ZLLentz Feb 14, 2025 •

edited

Loading