Skip to content

Route every event marker through push_marker() / subscribe_marker()#328

Draft
pellet wants to merge 4 commits into
NeuroTechX:masterfrom
pellet:feat/event-marker-abstraction
Draft

Route every event marker through push_marker() / subscribe_marker()#328
pellet wants to merge 4 commits into
NeuroTechX:masterfrom
pellet:feat/event-marker-abstraction

Conversation

@pellet

@pellet pellet commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

Every paradigm currently hand-rolls its own marker handling: an if self.eeg: block
that re-derives the timestamp, branches on self.eeg.backend == "muselsl" to wrap the
marker in a list, and then separately calls send_triggers() for self.devices. The
same boilerplate is copy-pasted across N170, SSVEP, eoec, etc.

This PR routes marker handling through a single push_marker() method on
BaseExperiment, backed by a small subscriber list. One call fans the marker out to the
EEG board, every auxiliary device, and any optional observers under one shared
timestamp
.

Net paradigm change (N170 shown):

# before — duplicated in every paradigm
if self.eeg:
    timestamp = time()
    if self.eeg.backend == "muselsl":
        marker = [self.markernames[label]]
    else:
        marker = self.markernames[label]
    self.eeg.push_sample(marker=marker, timestamp=timestamp)
if self.devices:
    self.send_triggers(self.markernames[label])

# after
self.push_marker(self.markernames[label], idx)

Motivation

  1. DRY — the muselsl → wrap-in-list branch lived in every paradigm. It now lives in
    exactly one place (EEG._muse_push_sample), where the pylsl-outlet requirement
    actually belongs. Paradigm code drops to a single line.
  2. Correct cross-stream timing — previously the EEG push and send_triggers() each
    minted their own time() (and send_triggers minted a fresh one per device inside
    its loop), so a single stimulus onset was stamped with several slightly different
    times across streams. push_marker() computes the timestamp once and shares it with
    every sink — important for ERP epoching and multi-device alignment.
  3. Extensibility — optional observers (photodiode, flip-time telemetry, eyetracker)
    can subscribe to marker onsets without editing any paradigm.

API

push_marker(marker, trial_idx=None) -> float
    # Fans the marker out to every subscriber under one shared timestamp; returns that timestamp
    # so the caller can record the same onset in its own bookkeeping.

subscribe_marker(callback, raise_on_error=True)
    # callback is any callable(marker, timestamp, trial_idx).
    # raise_on_error=True  (default) — essential recorders; an exception aborts the run.
    # raise_on_error=False           — optional telemetry; exceptions are logged, not raised.

Two built-in essential subscribers are registered in __init__: _write_marker_to_eeg
(the board) and _write_marker_to_devices (aux devices). Paradigms can add their own — e.g. eoec
subscribes its LSL Markers outlet and, when present, the serial trigger box.

Failure handling for optional subscribers is spam- and latency-safe: a broken subscriber's
traceback is logged once, subsequent failures are counted (a dict increment, no
formatting/IO on the presentation thread), and a per-subscriber total is reported at the
end of run().

Preserved contract (no change for device authors)

  • self.devices and its default are unchanged.
  • The device protocol is unchanged: subscribers still call
    dev.push_sample(marker=..., timestamp=...). Existing device objects work as-is.

Behavior changes to be aware of (review these)

  1. send_triggers() is removed. Its behavior is folded into the _write_marker_to_devices
    subscriber. In-repo callers (N170, SSVEP, eoec, p300) are updated. If external/downstream code calls
    send_triggers(), that's a breaking change — happy to keep a thin back-compat shim
    (send_triggers = lambda self, m: self.push_marker(m)) if preferred.
  2. Timestamps are now identical across streams where they previously differed by
    µs–ms. This is the intended fix, but it does change recorded marker timestamps.
  3. Two paradigms are not migrated hereauditory_oddball and pattern_reversal_vep
    still call eeg.push_sample() directly. Converting them is a mechanical follow-up.

SSVEP timing note

SSVEP now pushes the marker on the first flip of the flicker loop (guarded by a
marker_pushed flag) so the recorded onset is the true stimulus onset rather than one
frame early.

Tests

  • tests/test_push_marker.py — display-free unit coverage that runs headless in CI (no
    window): shared-timestamp fan-out, optional-subscriber isolation (both directions),
    log-once-per-subscriber anti-spam, and the muse scalar→[marker] wrap/back-compat.

pellet added 3 commits July 7, 2026 08:03
…ubscribe_marker()

Route all stimulus-marker emission through a single push_marker() on
BaseExperiment, backed by a subscriber list. One call fans the marker out to the
EEG board, every auxiliary device, and any optional observers under one shared
timestamp — replacing the per-paradigm boilerplate that re-derived the timestamp
and branched on eeg.backend to wrap muse markers.

- subscribe_marker(callback, raise_on_error=True): register any
  callable(marker, timestamp, trial_idx). raise_on_error=True aborts the run on
  failure (essential recorders); False logs-and-swallows (optional telemetry),
  logging each broken subscriber's traceback only once to keep per-onset log I/O
  off the presentation thread.
- push_marker(marker, trial_idx=None) -> float: emit under one shared timestamp
  and return it so callers can stamp their own bookkeeping.
- Built-in essential subscribers _emit_to_eeg / _emit_to_devices preserve the
  existing self.devices contract (dev.push_sample(marker=, timestamp=)).
- muse scalar->[marker] wrap now lives once in EEG._muse_push_sample instead of
  being duplicated across paradigms.
- eoec: LSL outlet + serial trigger box subscribe as inline lambdas; the serial
  write rides push_marker's shared timestamp (best-effort, raise_on_error=False).
- SSVEP emits on the first flip so the recorded onset is the true stimulus onset.

Removes BaseExperiment.send_triggers() (folded into _emit_to_devices).
Adds tests/test_push_marker.py.
…tent naming

- Convert p300 and n170 to push_marker(): drop the pre-abstraction self.eeg
  guards and p300's inline muse-wrap/manual timestamp (the wrap now lives in
  EEG._muse_push_sample). p300 was the last raw-push holdout.
- Rename built-in subscribers _emit_to_eeg/_emit_to_devices ->
  _write_marker_to_eeg/_write_marker_to_devices and purge "emit" from
  docstrings/comments in favour of "marker". Note "trigger" as the ERP synonym
  and leave a send_triggers breadcrumb in the push_marker docstring.
- Simplify test_push_marker.py (real construction via a fixture instead of
  object.__new__; one fake sink) and move the muse-wrap unit test to
  test_acquisition.py, alongside the other EEG-device tests.
…riber alias

Add MarkerSubscriber = Callable[[int, float, Optional[int]], None] and annotate
subscribe_marker(callback) and marker_subscribers with it, so the subscriber
contract is a single named type: autocomplete and static checks at the call site,
and Optional[int] surfaces that trial_idx may be None.
@pellet pellet marked this pull request as draft July 8, 2026 12:27
- conftest: skip tests/test_push_marker.py when psychopy is absent (it
  imports BaseExperiment -> devices.vr -> psychopy), matching the existing
  eegnb/experiments gate. Fixes test collection on the py-3.12/3.13
  streaming CI env.
- eoec: annotate self.outlet/self.serial as Optional[...] and bind locals
  before the subscribe_marker closures so mypy can narrow them. Fixes the
  two attr-defined errors in the typecheck job.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant