Scanspec 2 updates#212
Conversation
There was a problem hiding this comment.
Pull request overview
Ports missing scanspec 1.x motion-spec primitives into scanspec2 and formalizes centred-livetime trigger semantics, including extending Window.positions() to support both servo-rate sampling and trigger-instant sampling.
Changes:
- Add
Linspace.bounded, introduceRange+Range.bounded, and addEllipse/Polygonmasked-grid specs plusLinealias. - Update
TriggerPatterndocs to define centred-livetime semantics and explicitly allowlivetime=0.0. - Widen
Window.positions()to acceptfloat | TriggerPattern, with new unit tests and an ADR documenting the decision.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/scanspec2/specs.py |
Adds Linspace.bounded, Range, Ellipse, Polygon, and Line alias implementations. |
src/scanspec2/core.py |
Updates TriggerPattern docstring and extends Window.positions() to accept TriggerPattern. |
tests/scanspec2/test_specs.py |
Adds construction/validation tests for new spec primitives and aliases. |
tests/scanspec2/test_compile.py |
Adds compile-time behavior tests for new specs (Range, Ellipse, Polygon, bounded constructors). |
tests/scanspec2/test_core.py |
Adds behavioural tests for centred-livetime semantics and Window.positions() overload. |
docs/explanations/decisions/0006-trigger-pattern-centred-livetime-and-positions-signature.md |
ADR documenting centred-livetime semantics + Window.positions() signature change. |
Comments suppressed due to low confidence (2)
src/scanspec2/specs.py:369
- Range.bounded does not preserve the provided bounds when it collapses to a single frame (step >= abs(upper-lower)): it makes start≈stop and relies on nextafter, which causes LinearSource's fence/post boundaries to collapse around the midpoint rather than matching (lower, upper). Linspace.bounded handles the 1-point case by encoding the frame width in stop; Range.bounded should do something equivalent so fly-scan boundaries remain correct.
distance = abs(upper - lower)
direction = np.sign(upper - lower)
step = min(distance, abs(step))
half_step = step / 2 * direction
start = lower + half_step
stop = upper - half_step
if stop == start:
stop = np.nextafter(start, np.inf * direction)
return cast(Range[_BoundedAxisT], cls(cast(Any, axis), start, stop, step))
src/scanspec2/specs.py:1068
- Polygon.compile also ignores the snake flag for the same reason as Ellipse: snake is not applied to the fast axis and _eval_full_grid does not respect snaking, so snake=True has no effect on traversal order. To match the 1.x behavior, the masked point ordering needs to be generated with fast-axis snaking (boustrophedon) applied.
if self.vertical:
grid: Spec[AxisT, Never, Never] = x_range * y_range
else:
grid = y_range * x_range
all_pts = _eval_full_grid(grid.compile())
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| x_radius = abs(self.x_diameter) / 2 | ||
| eff_y_diam = ( | ||
| self.y_diameter if self.y_diameter is not None else abs(self.x_diameter) | ||
| ) | ||
| y_radius = abs(eff_y_diam) / 2 | ||
| eff_y_step = self.y_step if self.y_step is not None else self.x_step | ||
|
|
||
| x_range: Range[AxisT] = Range( | ||
| self.x_axis, | ||
| self.x_centre - x_radius, | ||
| self.x_centre + x_radius, | ||
| self.x_step, | ||
| ) | ||
| y_range: Range[AxisT] = Range( | ||
| self.y_axis, | ||
| self.y_centre - y_radius, | ||
| self.y_centre + y_radius, | ||
| eff_y_step, | ||
| ) | ||
|
|
||
| # Build grid: slow * fast (snake/vertical determine fast axis but do | ||
| # not change the set of masked points). | ||
| if self.vertical: | ||
| grid: Spec[AxisT, Never, Never] = x_range * y_range | ||
| else: | ||
| grid = y_range * x_range | ||
|
|
||
| all_pts = _eval_full_grid(grid.compile()) | ||
|
|
||
| x = all_pts[self.x_axis] - self.x_centre | ||
| y = all_pts[self.y_axis] - self.y_centre | ||
| mask = (2 * x / self.x_diameter) ** 2 + (2 * y / eff_y_diam) ** 2 <= 1 | ||
|
|
| # Build grid: slow * fast (snake/vertical determine fast axis but do | ||
| # not change the set of masked points). | ||
| if self.vertical: | ||
| grid: Spec[AxisT, Never, Never] = x_range * y_range | ||
| else: | ||
| grid = y_range * x_range | ||
|
|
||
| all_pts = _eval_full_grid(grid.compile()) | ||
|
|
| x_axis: AxisT = Field(description="Axis identifier for x.") | ||
| y_axis: AxisT = Field(description="Axis identifier for y.") | ||
| vertices: list[tuple[float, float]] = Field( | ||
| description="Ordered (x, y) vertices of the polygon." | ||
| ) | ||
| x_step: float = Field(gt=0, description="Grid spacing along x.") | ||
| y_step: float | None = Field( | ||
| default=None, | ||
| description="Grid spacing along y (defaults to x_step).", | ||
| ) | ||
| snake: bool = Field(default=False, description="Snake the fast axis.") | ||
| vertical: bool = Field(default=False, description="If True, y is the fast axis.") | ||
|
|
||
| @model_validator(mode="after") | ||
| def _fill_y_defaults(self) -> Self: | ||
| if self.y_step is None: | ||
| object.__setattr__(self, "y_step", self.x_step) | ||
| return self | ||
|
|
||
| def _eff_y_step(self) -> float: | ||
| return self.y_step if self.y_step is not None else self.x_step | ||
|
|
||
| def _poly_mask(self, x: np.ndarray, y: np.ndarray) -> np.ndarray: | ||
| """Even-odd ray-casting mask (matches 1.x algorithm exactly).""" | ||
| v1x, v1y = self.vertices[-1] | ||
| mask = np.full(len(x), False, dtype=np.bool_) | ||
| for v2x, v2y in self.vertices: | ||
| if v2y != v1y: |
| n_total = int(self.duration / dt) if dt > 0 else 1 | ||
| chunk = ( | ||
| int(max_duration / dt) | ||
| if max_duration is not None and dt > 0 | ||
| else n_total | ||
| ) | ||
| start = 0 | ||
| while start < n_total: | ||
| end = min(start + chunk, n_total) | ||
| indexes = np.linspace(start * dt, end * dt, end - start) | ||
| yield self._positions_fn(indexes) |
| # of that repeat's ½·deadtime leading edge. | ||
| centre_offset = tp.deadtime / 2 + tp.livetime / 2 | ||
| # Absolute timestamps of the midpoint of each repeat's livetime. | ||
| indexes = np.array([i * period + centre_offset for i in range(tp.repeats)]) |
coretl
left a comment
There was a problem hiding this comment.
This is the old version of what we discussed. We then had a followup where we discussed:
time_chunks(dt: float, max_size: int | None = None) -> Iterator[np.ndarray], starting at start_timepositions(indexes: np.ndarray) -> dict[str, np.ndarray]- truncating trigger patterns based on start_time
We also discussed about maybe turning start_time into a trigger pattern index
| sequencer table and position-compare triggering) require the detector's active | ||
| window to be centred on the nominal scan position. With `livetime → deadtime` | ||
| semantics the active window is leading-edge-aligned, which produces a | ||
| systematic position error proportional to `½·livetime`. |
There was a problem hiding this comment.
| systematic position error proportional to `½·livetime`. | |
| systematic position error proportional to `½·deadtime`. |
| semantics the active window is leading-edge-aligned, which produces a | ||
| systematic position error proportional to `½·livetime`. | ||
|
|
||
| **Zero livetime**: The tomography variable-gap ptychography use case requires |
There was a problem hiding this comment.
| **Zero livetime**: The tomography variable-gap ptychography use case requires | |
| **Zero livetime**: The variable-gap ptychography use case requires |
|
|
||
| ```python | ||
| [ | ||
| TriggerPattern(1, 0.0, half_gap), # leading spacer |
There was a problem hiding this comment.
A leading and trailing spacers are misleading. TriggerPattern(N, livetime, deadtime) is interpreted by ophyd-async as 1/2 deadtime then livetime then 1/2 deadtime. There is no need of spacers for this example.
You need the spacer for the ptychography example: [TriggerPattern(N1, livetime1, deadtime), TriggerPattern(1, 0.0, gap), TriggerPattern(N2, livetime2, deadtime)]
06398f7 to
2f3e917
Compare
* Add ophyd_async dev dependency and serialize non-JSON-serializable axes via repr - Add ophyd_async to the dev dependency group in pyproject.toml - Update Spec.serialize() to use mode='json' with fallback=repr so that non-JSON-serializable axes (e.g. ophyd_async Device objects) are converted to their repr string, making the output always JSON-serializable - Add tests for serializing a spec with an ophyd_async Device as axis and verifying the resulting dict is JSON-serializable * Fix ophyd-async dependency name and improve serialize() docstring - Use canonical distribution name ophyd-async (not ophyd_async) in pyproject.toml - Reword Spec.serialize() docstring to accurately describe the global repr() fallback for any non-JSON-serializable value, not just axes * Use name attribute as fallback in serialize before repr * fix type annotation * Add test for repr fallback when axis has no name attribute * Add axis assertion and json serializability check to ophyd_async device test
…indow.positions(TriggerPattern)
…sitions() signature
Replace start_time: float with _start_trigger_index: int. - Add _truncate_trigger_patterns() helper: walks the flat repeat sequence, splits mid-pattern, returns (remaining, consumed_duration) - Scan.__init__: start_time → start_trigger_index - Scan.with_start(window, trigger_index=0): new signature; raises ValueError if trigger_index > 0 and window has >1 TriggerGroup - Scan.__iter__: applies truncation on the first yielded window when _start_trigger_index > 0, constructing a new TriggerGroup to avoid mutating shared generator state - 7 new tests in test_core.py covering split, cross-pattern skip, spacer-as-repeat, duration reduction, and multi-group ValueError
2f3e917 to
9aff5db
Compare
| if window_idx == self._start_window and self._start_trigger_index > 0: | ||
| tgs = inner_window.trigger_groups | ||
| if len(tgs) > 1: | ||
| raise ValueError( | ||
| f"trigger_index > 0 requires exactly one TriggerGroup; " | ||
| f"window {window_idx} has {len(tgs)}" | ||
| ) | ||
| if tgs: | ||
| remaining, consumed = _truncate_trigger_patterns( | ||
| tgs[0].trigger_patterns, self._start_trigger_index | ||
| ) | ||
| inner_window.trigger_groups = [ | ||
| TriggerGroup( | ||
| detectors=tgs[0].detectors, | ||
| trigger_patterns=remaining, | ||
| ) | ||
| ] | ||
| inner_window.duration -= consumed |
| if skip_repeats == 0: | ||
| return list(patterns), 0.0 | ||
| consumed_duration = 0.0 | ||
| remaining = skip_repeats | ||
| for i, tp in enumerate(patterns): | ||
| if remaining <= 0: | ||
| return list(patterns[i:]), consumed_duration | ||
| if remaining >= tp.repeats: | ||
| consumed_duration += (tp.livetime + tp.deadtime) * tp.repeats | ||
| remaining -= tp.repeats | ||
| else: | ||
| consumed_duration += (tp.livetime + tp.deadtime) * remaining | ||
| suffix: list[TriggerPattern] = [ | ||
| TriggerPattern(tp.repeats - remaining, tp.livetime, tp.deadtime), | ||
| *patterns[i + 1 :], | ||
| ] | ||
| return suffix, consumed_duration | ||
| return [], consumed_duration |
| def _eval_full_grid(scan: Scan[Any, Any, Any]) -> dict[Any, np.ndarray]: | ||
| """Return all midpoints of a compiled scan as flat arrays. | ||
|
|
||
| Expands a multi-generator scan into a full Cartesian product, giving one | ||
| array per axis of length ``product(g.length for g in scan.generators)``. | ||
| Snake flags are ignored — the caller cares only about *which* points | ||
| exist, not their traversal order. | ||
| """ | ||
| per_axis: dict[Any, np.ndarray] = {} | ||
| outer_len = 1 | ||
| for gen in scan.generators: | ||
| inner_len = gen.length | ||
| midpt_idx = np.arange(inner_len, dtype=float) + 0.5 | ||
| pts = gen.setpoints(midpt_idx) | ||
| # Each already-collected outer axis repeats inner_len times. | ||
| for ax in list(per_axis.keys()): | ||
| per_axis[ax] = np.repeat(per_axis[ax], inner_len) | ||
| # Each new inner axis tiles outer_len times. | ||
| for ax, arr in pts.items(): | ||
| per_axis[ax] = np.tile(arr, outer_len) | ||
| outer_len *= inner_len | ||
| return per_axis |
| # Build grid: slow * fast (snake/vertical determine fast axis but do | ||
| # not change the set of masked points). | ||
| if self.vertical: | ||
| grid: Spec[AxisT, Never, Never] = x_range * y_range | ||
| else: | ||
| grid = y_range * x_range | ||
|
|
| if self.vertical: | ||
| grid: Spec[AxisT, Never, Never] = x_range * y_range | ||
| else: | ||
| grid = y_range * x_range | ||
|
|
| if isinstance(dt, TriggerPattern): | ||
| tp = dt | ||
| period = tp.livetime + tp.deadtime | ||
| # Centre of each repeat: ½·deadtime + ½·livetime from the start | ||
| # of that repeat's ½·deadtime leading edge. | ||
| centre_offset = tp.deadtime / 2 + tp.livetime / 2 | ||
| # Absolute timestamps of the midpoint of each repeat's livetime. | ||
| indexes = np.array([i * period + centre_offset for i in range(tp.repeats)]) | ||
| chunk_dur = max_duration if max_duration is not None else np.inf | ||
| start = 0 | ||
| while start < len(indexes): | ||
| # Find how many repeats fit within max_duration from first index | ||
| if chunk_dur is not np.inf: | ||
| t0 = indexes[start] | ||
| end = start | ||
| while end < len(indexes) and indexes[end] - t0 < chunk_dur: | ||
| end += 1 | ||
| if end == start: # at least one per chunk | ||
| end = start + 1 | ||
| else: | ||
| end = len(indexes) | ||
| yield self._positions_fn(indexes[start:end]) | ||
| start = end | ||
| else: |
…d fact; add table-rewrite citation; qualify chained-tables claim
…§4 and consequences §5
|
|
||
| **Pause/resume mechanism** (proposed extension to existing ophyd-async PandA | ||
| behaviour): the existing [`ScanSpecSeqTableTriggerLogic`](https://github.com/bluesky/ophyd-async/blob/2b648bc925c026a185aaba0cf54533db2a725448/src/ophyd_async/fastcs/panda/_fly_logic.py#L91) already places | ||
| `BITA=0` / `BITA=1` gate rows at **window boundaries** for motion-controller |
|
|
||
| To pause, Bluesky sets BITA=0; PandA finishes its current repeat and stalls at | ||
| the next checkpoint gate row in `WAIT_TRIGGER`. Bluesky polls `TABLE_LINE` and | ||
| `LINE_REPEAT` ([PandA SEQ docs — Fields](https://pandablocks-fpga.readthedocs.io/en/latest/build/seq_doc.html#fields)) |
|
|
||
| ### A1 — Multi-entry root lists are manual-construction-only | ||
|
|
||
| `_bake_trigger_nodes` (called by all compiled `Spec` subclasses) always |
| iterates over `trigger_nodes` (including `_truncate_trigger_nodes` and | ||
| `_compute_duration`) is written to handle multi-entry lists correctly for | ||
| robustness, but no compiled `Spec` subclass is responsible for producing | ||
| them. |
There was a problem hiding this comment.
No zipping of completely unrelated parallel trigger streams with no common checkpoint base
| ([PandA SEQ docs — External trigger sources](https://pandablocks-fpga.readthedocs.io/en/latest/build/seq_doc.html#external-trigger-sources)), | ||
| so the consumer must not collapse N repeats into a single `REPEATS=N` data | ||
| row — doing so reduces N checkpoints to one. A valid minimal encoding is a | ||
| two-row sub-table `[gate_row (TRIGGER=BITA=1, REPEATS=1), data_row |
There was a problem hiding this comment.
Will never occur in reality, expand instead the seq table example we wrote on the board
|
|
||
| ### A3 — PandA supports a maximum of two chained SEQ tables | ||
|
|
||
| A standard PandA has two SEQ sequencer block instances, which sets a maximum |
There was a problem hiding this comment.
Correct, but we can do exactly 2 levels of nesting in a single SEQ block
Port specs from scanspec 1.x, implement intra-window pause/resume logic
Closes #196
Closes #214