feat: add keep_shutter_open logic

[fdrgsp]

@tlambert03 for the MDAEvent keep_shutter_open what do you think about using a logic similar to the autoshutter?

This PR gives an example of that.

Briefly, the MDASequence has a shutter_plan which is a ShutterOpenAxes class that can be used to specify the axes where the shutter should be kept open. As the AutoFocusPlan, ShutterOpenAxes has a should_keep_open method that return True if the specified axes have changed from the previous event.

So we can simply call shutter_plan.should_keep_open(event) before yielding the event to set the MDAEvent keep_shutter_open attribute to True or False.

if shutter_plan.should_keep_open(event):
    event = event.copy(update={"keep_shutter_open": True})

[codecov[bot]]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.05 :tada:

Comparison is base (e50aa35) 96.35% compared to head (873b68c) 96.41%.

:exclamation: Current head 873b68c differs from pull request most recent head 75ebd59. Consider uploading reports for the commit 75ebd59 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #97 +/- ## ========================================== + Coverage 96.35% 96.41% +0.05% ========================================== Files 12 13 +1 Lines 714 753 +39 ========================================== + Hits 688 726 +38 - Misses 26 27 +1 ``` | [Impacted Files](https://app.codecov.io/gh/pymmcore-plus/useq-schema/pull/97?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pymmcore-plus) | Coverage Δ | | |---|---|---| | [src/useq/\_\_init\_\_.py](https://app.codecov.io/gh/pymmcore-plus/useq-schema/pull/97?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pymmcore-plus#diff-c3JjL3VzZXEvX19pbml0X18ucHk=) | `100.00% <100.00%> (ø)` | | | [src/useq/\_mda\_event.py](https://app.codecov.io/gh/pymmcore-plus/useq-schema/pull/97?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pymmcore-plus#diff-c3JjL3VzZXEvX21kYV9ldmVudC5weQ==) | `86.66% <100.00%> (+0.22%)` | :arrow_up: | | [src/useq/\_mda\_sequence.py](https://app.codecov.io/gh/pymmcore-plus/useq-schema/pull/97?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pymmcore-plus#diff-c3JjL3VzZXEvX21kYV9zZXF1ZW5jZS5weQ==) | `98.42% <100.00%> (+0.07%)` | :arrow_up: | | [src/useq/\_shutter\_plan.py](https://app.codecov.io/gh/pymmcore-plus/useq-schema/pull/97?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pymmcore-plus#diff-c3JjL3VzZXEvX3NodXR0ZXJfcGxhbi5weQ==) | `100.00% <100.00%> (ø)` | | ... and [6 files with indirect coverage changes](https://app.codecov.io/gh/pymmcore-plus/useq-schema/pull/97/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pymmcore-plus)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[tlambert03]

that confuses me too... What does "during this event" mean? Aren't shutters almost always open during an event? Remember, an event tends to be a single snap. I think what you're trying to express is that the shutter should remain open even when this event is done. For example, if someone wanted to keep it open throughout a z stack (most common thing to do) then they would want the shutter to remain open during many events (not just one). I think you know that, but the wording is still unclear.

On Jul 17, 2023, at 4:02 PM, federico gasparoli @.***> wrote:

@fdrgsp commented on this pull request.

In src/useq/_mda_event.py https://github.com/pymmcore-plus/useq-schema/pull/97#discussion_r1265840438:

@@ -115,6 +115,8 @@ class MDAEvent(UseqModel): The action to perform for this event. By default, [useq.AcquireImage][]. Example of another action is [useq.HardwareAutofocus][] which could be used to perform a hardware autofocus.

• keep_shutter_open : bool
• Keep the shutter open between during this event. By default, False. it's a mistake, it should be updated as:

If True, the shutter should be kept open during this event. By default, False.

— Reply to this email directly, view it on GitHub https://github.com/pymmcore-plus/useq-schema/pull/97#discussion_r1265840438, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMI52KDHR4R4SGFHDJK77TXQWK6LANCNFSM6AAAAAA2NL3ZP4. You are receiving this because you were mentioned.

[tlambert03]

i think i'm ok with this. I'm a little hesitant to keep adding "plans" to the top level of the MDASequence schema. Since this is rather similar to autofocus (with the primary difference being it doesn't add events, it modifies events...) I'd like to think for a moment about what the general patterns are and think about whether this can be done in a slightly more general way

[fdrgsp]

that confuses me too... What does "during this event" mean? Aren't shutters almost always open during an event? Remember, an event tends to be a single snap. I think what you're trying to express is that the shutter should remain open even when this event is done. For example, if someone wanted to keep it open throughout a z stack (most common thing to do) then they would want the shutter to remain open during many events (not just one). I think you know that, but the wording is still unclear. …

Yes I agree. I thought that every event should have a keep_shutter_open bool so that an engine will check if it has to set the shutter to the open state of the closed state.

for example, in case of our pymmcore_plus engine we could use the FullPMDAEngine and have something like:

def teardown_event(self, event: MDAEvent) -> None:
        if self._use_autoshutter and event.keep_shutter_open:  # type: ignore
            self._mmc.setAutoShutter(False)
            self._mmc.setShutterOpen(True)

and then, at the end of a sequence we can reset the shutter to its original state (by original I mean the state when the sequence started).

def teardown_sequence(self, sequence: MDASequence) -> None:
        """Teardown state of system (hardware, etc.) after `sequence`."""
        if self._use_autoshutter:
            self._mmc.setAutoShutter(True)
            self._mmc.setShutterOpen(False)

[fdrgsp]

i think i'm ok with this. I'm a little hesitant to keep adding "plans" to the top level of the MDASequence schema. Since this is rather similar to autofocus (with the primary difference being it doesn't add events, it modifies events...) I'd like to think for a moment about what the general patterns are and think about whether this can be done in a slightly more general way

Sure! I had the same feeling but I thought it could a starting point.

[tlambert03]

Yes I agree. I thought that every event should have a keep_shutter_open bool so that an engine will check if it has to set the shutter to the open state of the closed state.

yep. I agree with all of this, my point is just that the docs are confusing. It's not what happens during the event, it's what happens after it. The docs here should make it super clear to someone implementing the engine exactly what they are supposed to do. I think we're in agreement about what keep_shutter_open means, but it needs to be made clearer. I would say "If keep_shutter_open is True, the illumination shutter should be left open after the event has been executed, otherwise it should be closed."

[tlambert03]

having another look at this... i'm not sure we want to reuse the same logic from autofocus here. Or, maybe we do but I'm not getting it yet... Can you give me an example of how you'd express "keep the shutter open while moving z", in a multi-channel, z stack? Particularly because this is more of a thing that persists across events (or rather, an instruction for how to leave the state), it feels different.

Also shouldn't we be ensuring that we're only keeping the shutter open if the "keep open" axis is the fastest moving one? For example, you can't keep the illumination shutter open if the channel is changing. So there appear to be more limitations here than autofocus. In general, does it ever make sense to have the kept-open axis be slower moving than channel?

[tlambert03]

i think it would be helpful to delete the test_keep_shutter_open file and make some clearer tests, without just reusing the autofocus file. (not sure for the sake of removing duplication, but since I'm not sure it's the same thing)

[fdrgsp]

Can you give me an example of how you'd express "keep the shutter open while moving z", in a multi-channel, z stack

So basically I was thinking that if you want to have the shutter open during a z stack, then you have to specify the "z" in the shutter_plan shutter_plan={"axes":("z",)}:

running this:

s = MDASequence(
    channels=["DAPI", "FITC"],
    stage_positions=[(0,0,10)],
    shutter_plan={"axes":("z",)},
    z_plan={"range": 1.0, "step": 0.5}
)

for e in s:
    print(f"INDEX:' {e.index}, SHUTTER_OPEN: {e.keep_shutter_open}")

results in this:

INDEX: {'p': 0, 'c': 0, 'z': 0} SHUTTER_OPEN: True
INDEX: {'p': 0, 'c': 0, 'z': 1} SHUTTER_OPEN: True
INDEX: {'p': 0, 'c': 0, 'z': 2} SHUTTER_OPEN: True
INDEX: {'p': 0, 'c': 1, 'z': 0} SHUTTER_OPEN: True
INDEX: {'p': 0, 'c': 1, 'z': 1} SHUTTER_OPEN: True
INDEX: {'p': 0, 'c': 1, 'z': 2} SHUTTER_OPEN: True

because the first axes changing is the "z" axes.

if you change the axis_order to tpgzc

s = MDASequence(
    axis_order="tpgzc",
    channels=["DAPI", "FITC"],
    stage_positions=[(0,0,10)],
    shutter_plan={"axes":("z",)},
    z_plan={"range": 1.0, "step": 0.5}
)

then you get

INDEX:' {'p': 0, 'c': 0, 'z': 0}, SHUTTER_OPEN: True
INDEX:' {'p': 0, 'c': 1, 'z': 0}, SHUTTER_OPEN: False
INDEX:' {'p': 0, 'c': 0, 'z': 1}, SHUTTER_OPEN: True
INDEX:' {'p': 0, 'c': 1, 'z': 1}, SHUTTER_OPEN: False
INDEX:' {'p': 0, 'c': 0, 'z': 2}, SHUTTER_OPEN: True
INDEX:' {'p': 0, 'c': 1, 'z': 2}, SHUTTER_OPEN: False

because the first axes changing is the channel one (basically is like not having set the shutter_plan). So in this case, if you want to have the shutter always open you just have to set the shutter_plan axes to both ("c", "z") (or simply ("c",)).

This is what I thought it could make sense :) but probably there is a different logic that works!

[fdrgsp]

i think it would be helpful to delete the test_keep_shutter_open file and make some clearer tests, without just reusing the autofocus file. (not sure for the sake of removing duplication, but since I'm not sure it's the same thing)

I used the same logic of the autofocus test first to have something fast to use as tests :) but also because since we have an extra autofocus event, I wanted to make sure that the shutter_plan works also during the autofocus events.

[tlambert03]

yes, but practically speaking...

INDEX: {'p': 0, 'c': 0, 'z': 0} SHUTTER_OPEN: True
INDEX: {'p': 0, 'c': 0, 'z': 1} SHUTTER_OPEN: True
INDEX: {'p': 0, 'c': 0, 'z': 2} SHUTTER_OPEN: True   # <- shouldn't this be False?
INDEX: {'p': 0, 'c': 1, 'z': 0} SHUTTER_OPEN: True
INDEX: {'p': 0, 'c': 1, 'z': 1} SHUTTER_OPEN: True
INDEX: {'p': 0, 'c': 1, 'z': 2} SHUTTER_OPEN: True   # <- shouldn't this be False?

# shouldn't these ALL be false?
INDEX:' {'p': 0, 'c': 0, 'z': 0}, SHUTTER_OPEN: True
INDEX:' {'p': 0, 'c': 1, 'z': 0}, SHUTTER_OPEN: False
INDEX:' {'p': 0, 'c': 0, 'z': 1}, SHUTTER_OPEN: True
INDEX:' {'p': 0, 'c': 1, 'z': 1}, SHUTTER_OPEN: False
INDEX:' {'p': 0, 'c': 0, 'z': 2}, SHUTTER_OPEN: True
INDEX:' {'p': 0, 'c': 1, 'z': 2}, SHUTTER_OPEN: False

[tlambert03]

i think the point here is that, unlike autofocus events, leaving the shutter open is intrinsically linked to the channel axis. It doesn't ever make sense to leave the shutter open if the next image can't be acquired with that same "open shutter". So, your predicate here really needs awareness of whether the next event is using the same channel config as the current one