StandardDetector.stage behavior with writer and trigger info

[thopkins32]

Hello, while implementing #726, I ran into an issue where you can't really set the TriggerInfo for a simple count plan easily.

For example, if I try to do

trigger_info = TriggerInfo(number_of_events=1, exposures_per_event=5)
# Prepare the detector to use capture 5 exposures in a single event
RE(bps.prepare(det, trigger_info))
# EXPECTED: Gather data for 5 events. 5 events x 5 exposures per event = 25 total
# ACTUAL: 5 total since trigger_info is unset in stage()
RE(bp.count([det], num=5))

The only way around this is to create a custom plan that calls stage() before bps.prepare and make it into a single RE call.

This begs the question, should we really be unsetting TriggerInfo during stage()?
Should we be closing the file writer during stage()?

https://github.com/bluesky/ophyd-async/blob/main/src/ophyd_async/core/_detector.py#L230-L231

Acceptance Criteria

• My example above runs and produces 25 exposures as output

[coretl]

Hmm, I'm not sure about this. We use stage at the moment to say "put into a known good state", which should close the file writer ready for a new file, but I'm not sure whether it should unset the TriggerInfo.

• Pro: There is prior art in that TriggerInfo(livetime=None) means "take the last configured exposure from the detector", which will also survive stage
• Con: If you prepare once to try things out, then someone else goes down to run the count plan, it might be surprising that it now does something different, especially as there's nothing corresponding to exposures_per_event in EPICS to inspect

@danielballan @tacaswell @jwlodek @mrakitin thoughts?

An alternative would be to add support for prepare to count in someway, either with an extra prepare argument, or a custom per_shot plan...

[thopkins32]

especially as there's nothing corresponding to exposures_per_event in EPICS to inspect

In the case of a step scan, the equivalent is self.driver.num_images https://github.com/bluesky/ophyd-async/blob/main/src/ophyd_async/epics/adcore/_core_logic.py#L46

I do see your point about stage putting the detector into a known good state. I agree that it should stay the way it currently is. That being said, generally what prepare could do is to either read or write EPICS PVs. If the TriggerInfo field is None, then we read back the value from EPICS, otherwise we set the PV value.

In my head it would look something like this for a step scan:

# Set EPICS PVs to match the values in `TriggerInfo`
trigger_info = TriggerInfo(number_of_events=1, exposures_per_event=5)
RE(bps.prepare(det, trigger_info)) 
# Run a count reading the current EPICS PVs readback values
RE(bp.count([det], num=5))
|-- det.stage() --> unset trigger info
|-- det.prepare() --> set default trigger info with all `None`s
|-- det.trigger()
|-- det.prepare()
|-- det.trigger()
|-- ...
|-- det.unstage() --> (unset trigger info here?)

So a user would be able to explicitly call det.prepare via the plan stub with the desired configuration. This configuration won't be hidden from the user because it will be visible at the EPICS level. Then each step of the scan will read the PVs to get the values for the scan.

[coretl]

In the case of a step scan, the equivalent is self.driver.num_images https://github.com/bluesky/ophyd-async/blob/main/src/ophyd_async/epics/adcore/_core_logic.py#L46

That only works for a step scan, for a fly scan that number will be the total number of images in the scan, so we wouldn't want to read that back for use as exposures_per_event for the next step scan...

There is also num_exposures, but that doesn't do the right thing either, that takes 5 exposures at the detector level and packs them into 1 exposure at the EPICS level, while exposures_per_event takes 5 exposures at the EPICS level and packs them into 1 bluesky event.

[thopkins32]

so we wouldn't want to read that back for use as exposures_per_event for the next step scan...

For step scans I think we have to be able to read it back from EPICS. If someone manually configures the self.driver.num_images they would expect that value to stick around. I see why this can't be the case for a fly scan.

For example,

def custom_count(det, exposures_per_event = 5):
    # Set the exposures_per_event at the EPICS level
    yield from bps.mv(det.controller.driver.num_images, exposures_per_event)
    # EXPECTED: Collect `exposures_per_event` number of images for a single event
    # ACTUAL: Always collects 1 image for a single event
    yield from bp.count([det])

RE(custom_count(det))

The above example currently doesn't work as I would expect. In the bp.count, det.trigger calls the det.prepare method is called and resets the det.controller.driver.num_images to 1 before collecting images. You would have to use bps.prepare in place of bps.mv for this to work because it would cache the TriggerInfo in the detector.

Should we use TriggerInfo for step scans at all? Maybe we can avoid these problems by separating the logic a bit more.

[thopkins32]

Following up on our discussion in the bluesky call today, I think that we should try our best to respect the current EPICS configuration as the default behavior. From a user's perspective, it can be convenient for diagnostic and small experiments to do some configuration work manually (like setting self.driver.num_images in CS Studio), then running a standard bluesky plan.

I foresee it being frustrating for users to see their configuration wiped out when they run a standard bluesky plan like count.

ophyd sync tends to repsect the current EPICS configuration in my experience (definitely for the equivalent of self.driver.num_images).

What this means in practice is up for discussion. I might be in favor of making TriggerInfo + prepare optional altogether unless there is a real requirement I am missing.

[coretl]

I foresee it being frustrating for users to see their configuration wiped out when they run a standard bluesky plan like count.

As far as I can see we have two options:

1. Make trigger() without prepare() use num_images from EPICS. This means that if you use the detector in a flyscan for 1000 exposures, then run a count plan, it will take 1000 exposures per event. If you want to ensure count always works the same way, you need to wrap it in a custom plan, or call bps.mv(det.drv.num_images, 1) before.
2. Make trigger() without prepare() override num_images from EPICS. This means that no matter what state your detector is in, it will always take 1 exposure per event. If you want to do count with a different number of exposures per event, you need to wrap it in a custom plan.

Personally I favour 2. I think using a single EPICS field as the source of exposures_per_event and the destination of total_number_of_exposures is fragile, and will lead to more surprises and frustrations at unpredictable times...

What this means in practice is up for discussion. I might be in favor of making TriggerInfo + prepare optional altogether unless there is a real requirement I am missing.

I'm not sure what you mean, isn't it already optional? You can stage, then trigger without needing prepare in between. It will respect all EPICS variables apart from num_images which it always overwrites...

[thopkins32]

@coretl @jwlodek I wanted to raise this issue again since I have been working on an ADEiger ophyd-async implementation for NSLS-II's CDI beamline: NSLS2/cditools#9

In my unit test with a fake count plan, if I set mock_eiger_detector.num_images to a value greater than 1, then it gets overwritten by the default value of TriggerInfo.exposures_per_event (which is currently 1).

There are many cases where someone would want to take a burst of images inside of a single Bluesky event but it doesn't seem possible to do so using standard Bluesky plans.

Restricting this discussion to step-scans only, I think we should either:

1. Abandon the use of TriggerInfo for step scans, so prepare doesn't have to be called
2. Make every field of TriggerInfo optional. If a field is None, then it reads the current PV value. If it is not None, then it writes the value to that PV.

I favor option 2 because it allows both step-scans and flyscans to be configured either by TriggerInfo or beforehand by EPICS directly.

[coretl]

Either option requires you to make a decision: if the user runs count then do they want num_images from EPICS or 1.

Let us consider a sequence:

• num_images is set to 2 using phoebus
• count plan is run
• flyscan is run with total_number_of_exposures = 1000
• count plan is run

If you choose EPICS as the source of truth for count plan then you get:

• 2 images per event for the first count
• 1000 images per event for the second count

This seems wrong to me.

If you choose 1 as the default you get:

• 1 images per event for the first count
• 1 images per event for the second count

This seems more intuitive to me.

If you want to set exposures_per_event to 2, then I suggest we make a plan stub in opyhd_async suitable for passing to count:

def make_custom_per_shot(exposures_per_event = 1, exposure = None):
    @plan
    def custom_per_shot(detectors: Sequence[StandardReadable]) -> MsgGenerator[None]:
        yield from bps.checkpoint()
        for det in detectors:
            yield from bps.prepare(det, TriggerInfo(exposures_per_event=exposures_per_event, livetime=exposure))
        yield from bps.wait()
        yield from bps.trigger_and_read(detectors)
    return custom_per_shot

RE(bp.count([det], make_custom_per_shot(2)))

[thopkins32]

Let us consider a sequence:

• num_images is set to 2 using phoebus
• count plan is run
• flyscan is run with total_number_of_exposures = 1000
• count plan is run

In my mind, it makes more sense that the following occurs:

• 2 images per event for the first count
• 1000 images per event for the second count
• 2 images per event for the second count

The 2 sticks around because this is what was explicitly set in the control screen.

At each execution of the plan count, ophyd-async should revert any changes made to the PVs back to what they originally were. Many beamlines have steady-state PV configurations where things are not being run. They expect ophyd to set the values it needs and put them back when the scan is complete.

Similar to how stage_sigs works in ophyd sync.