pymmcore-plus / useq-schema

An implementation agnostic schema for describing a sequence of events during a multi-dimensional imaging acquisition.
https://pymmcore-plus.github.io/useq-schema/
BSD 3-Clause "New" or "Revised" License
15 stars 5 forks source link

Position plan timing resets for each position #187

Open wl-stepp opened 4 days ago

wl-stepp commented 4 days ago

I will open a new issue for this, but it's very similar to #149

On useq-schema-0.4.7:

>>> import useq                                                                                       
>>> seq = useq.MDASequence(stage_positions=[(100,100), (0,0)], time_plan={"interval": 1, "loops": 2}, axis_order='ptgcz')
>>> list(seq)
[MDAEvent(index=mappingproxy({'t': 0, 'p': 0}), min_start_time=0.0, x_pos=100.0, y_pos=100.0), 
MDAEvent(index=mappingproxy({'t': 1, 'p': 0}), min_start_time=1.0, x_pos=100.0, y_pos=100.0), 
MDAEvent(index=mappingproxy({'t': 0, 'p': 1}), min_start_time=0.0, x_pos=0.0, y_pos=0.0), 
MDAEvent(index=mappingproxy({'t': 1, 'p': 1}), min_start_time=1.0, x_pos=0.0, y_pos=0.0)]

See how the min_start_time resets. With this the first position runs fine, but the later ones will run at full speed.

wl-stepp commented 4 days ago

I think the problem with integration in pymmcore-plus might be a bit deeper. For fast acquisitions on larger samples, this could still be a problem even if the counter would not reset. Maybe the timer in pymmcore-plus should actually reset for a new position after the position is set? Might be worth thinking about a little more.

tlambert03 commented 4 days ago

just to confirm, the goal here is to run a complete time lapse at a given position before moving on to the next position right? (I can definitely see how the current min_start_time strategy is insufficient and might require awareness on the acquisition engine as well)

tlambert03 commented 4 days ago

@wl-stepp, on the useq-schema side, what do you think the MDAEvent object should look like in this case? the most straightforward correction here could be to just offset p1 to have a min_start time at least as high as the last event in the previous position:

[MDAEvent(index=mappingproxy({'t': 0, 'p': 0}), min_start_time=0.0, x_pos=100.0, y_pos=100.0), 
MDAEvent(index=mappingproxy({'t': 1, 'p': 0}), min_start_time=1.0, x_pos=100.0, y_pos=100.0), 
MDAEvent(index=mappingproxy({'t': 0, 'p': 1}), min_start_time=1.0, x_pos=0.0, y_pos=0.0), 
MDAEvent(index=mappingproxy({'t': 1, 'p': 1}), min_start_time=2.0, x_pos=0.0, y_pos=0.0)]

That's slightly better, but would still be insufficient in terms of actually yielding the desired result.

We could could also allow an event to "reset" t0:

[MDAEvent(index=mappingproxy({'t': 0, 'p': 0}), min_start_time=0.0, x_pos=100.0, y_pos=100.0), 
MDAEvent(index=mappingproxy({'t': 1, 'p': 0}), min_start_time=1.0, x_pos=100.0, y_pos=100.0), 
MDAEvent(index=mappingproxy({'t': 0, 'p': 1}), min_start_time=0.0, x_pos=0.0, y_pos=0.0, reset_timer=True), 
MDAEvent(index=mappingproxy({'t': 1, 'p': 1}), min_start_time=1.0, x_pos=0.0, y_pos=0.0)]

other ideas? basically, how do you think this should look on the useq side, such that we can implement the proper behavior on the engine side?

wl-stepp commented 3 days ago

just to confirm, the goal here is to run a complete time lapse at a given position before moving on to the next position right? (I can definitely see how the current min_start_time strategy is insufficient and might require awareness on the acquisition engine as well)

Yes.

I think the underlying question might be who should know about some of the timing constraints here, right? I like the reset_timer idea, that way the acquisition engine can also decide when exactly to reset the timer. For a long setup it could reset the timer after it has moved to the new position etc. for example. The user then gives up tight timing from the last event before and the event with the reset_timer flag. I think that's a good option to have for many applications. Our adaptive acquisitions might want to have tight timing up to a trigger but then do something more time consuming and reset the timer with the last event.

Yeah, sounds good to me... How that is reflected in the metadata might be interesting to discuss, but if you give up tight timing across these events you might not be so interested in offsets anyways... And how downstream things might think that the time in the engine is monotonically rising... should there be a timerResetEvent?

tlambert03 commented 1 day ago

as i think about it now, i'm imagining that reset_timer=True is an instruction that all future min_start_times encountered in the sequence should be relative to start time of the the last seen MDAEvent with a timer reset. in terms of implementing it in the engine, I would imagine:

would that all work for you? While we're at it, can you foresee any future needs that this would not meet (such as additional timers, ids, etc...)

wl-stepp commented 1 day ago

Yeah, that sounds great. At the moment I can't think of anything that this concept would not cover. It should be possible to have additional timers if needed inside the adaptive acquisition framework for example, not the engine.