Re-resolve a Slicer selection when the axis values change

[shijiegu]

SlicerTransformer caches the indices a selection resolves to and rebuilds them only when _hash_message changes, which keys on the message key and the channel count. For a positional selection that is complete: nothing else can affect the answer. For a label, regex or field selection it is not, because those resolve against the coordinate values, and a source can rename, reorder or swap out channels without changing how many it sends -- e.g. a device reconfigured mid-session.

In concrete example:

selection "Ch7"
axis ['Ch5','Ch7','Ch10']  -> reports 'Ch7', data is position 1 = Ch7
axis ['Ch1','Ch5','Ch10']  -> reports 'Ch7', data is position 1 = Ch5
axis ['Ch1','Ch2','Ch3']   -> reports 'Ch7', data is position 1 = Ch2

Nothing raises this mismatch.

Solution: Include the coordinate data in the hash when the selection can consult it.

[cboulay]

Thanks for this — the bug is real and the diagnosis in the description is exactly right. A cache of resolved indices keyed on (key, n_ch) is unsound the moment those indices come from matching coordinate values, and the failure mode is the nasty kind: not a crash, but one channel's samples emitted under another channel's label, which downstream has no way to detect.

Two things before this can land: one blocking correctness gap, and a question about which house policy Slicer should follow.

The policy question

Chad flagged that AffineTransformTransformer solved something similar in _hash_message. Worth being explicit that it solved the opposite problem: group_spec_fingerprint is deliberately O(1) and folds in only a "does this field exist" boolean. There's a test named for the concession — test_common_rereference_field_values_change_is_not_detected:

Intentional concession: a live bank remap at fixed key + channel count is NOT re-derived… A genuine remap on real hardware arrives with a new key or channel count.

So copying affinetransform would close this PR, not fix it. But there is precedent for your direction too: ConcatTransformer._fingerprint already does hash(ax.data.tobytes()). The codebase holds both policies; the question is which one Slicer belongs to.

I think yours, for two reasons:

• Cost asymmetry. channel_groups defaults to None, so affinetransform's fingerprint returns () and the concession costs nothing for almost everyone. For Slicer, label selection is the primary use case, so the concession would be the default behaviour.
• Failure mode. A stale grouping is arithmetic on the wrong partition — wrong numbers, but the labels stay honest. A stale Slicer index is a provenance error that propagates silently. Worth a microsecond.

To make that split legible rather than accidental, I've written coord_value_fingerprint for util/channels.py as the explicit value-sensitive counterpart (not pushed yet — say the word and I'll open it as a PR against dev so this can build on it), with both docstrings cross-referencing each other. It handles the sharp edges below; see the notes on _hash_message.

Perf

Since the concern with folding values in is per-message cost, I measured it — benchmarks/benchmark_axis_fingerprint.py, same unpushed branch. 256 channels, full ezmsg-blackrock ChannelMap metadata (8 fields, 108 B itemsize, 27.6 kB), Apple M-series:

strategy	µs/msg	vs. baseline
hash((key, n_ch)) — baseline today	0.05	—
group_spec_fingerprint (affinetransform)	0.28	+0.23
hash(tobytes()) — this PR	5.03	+4.98
crc32 over the same bytes	0.95	+0.90
coord_value_fingerprint(msg, 'ch', None)	1.15	+1.10
coord_value_fingerprint(msg, 'ch', ('bank',))	0.90	+0.86
concat.py's _fingerprint (×2 inputs)	11.15	+11.10

For scale: +4.98 µs/msg is 0.5% of a core at 1 kHz and ~10× the data[:, :128] copy the hash is guarding. So the PR's cost is affordable — this isn't a reason to reject it — but it's 4× more than it needs to be, and it grows: at 2048 channels hash(tobytes()) is 42 µs vs 7 µs.

Three mechanical findings behind that gap, all in the benchmark:

1. The copy is cheap; siphash is not. tobytes() runs at ~94 GB/s, hash() over the result at ~5.5 GB/s. zlib.crc32 takes the array buffer directly at ~29 GB/s — 5.3× cheaper end to end. (Tradeoff: 32-bit, so a collision is a missed reset; dtype and shape ride along to absorb most of that.)
2. Asking numpy for a subset of fields is a trap. arr[['array','bank']] returns a view that keeps the original 108 B itemsize, so .tobytes() materialises all 27.6 kB — two of eight fields costs more than all eight (5.15 µs vs 5.03 µs). Fields have to be digested one at a time.
3. Narrowing to one field isn't reliably a speedup. Extracting is a strided gather (7–20 GB/s) against one contiguous read (29 GB/s), so a wide field loses: label (U16, 59% of the itemsize) costs 2.57 µs against 1.15 µs for the whole axis. The reason to narrow is invalidation correctness — see below — not speed.

Also worth knowing if you want it later: if a source attaches the same axis object to every message, an is-check fast path is 0.019 µs vs 0.95 µs. That needs transformer state, so it can't live in a pure helper, but SlicerTransformer has _state.

Housekeeping

ruff format --check fails on this branch — trailing whitespace at slicer.py:199 and 206.

[cboulay]

Blocking: this shortcut preserves the exact bug the PR fixes.

_state.new_axis is built in _reset_state from message.axes[axis].data[self._state.slice_] — the axis values — and it is cached alongside the indices. So even for a purely positional selection, the cached output axis goes stale when the labels change. Skipping the value hash here means nothing catches it.

Verified by grafting this branch's _hash_message and _resolves_by_position onto the transformer:

selection "0:2"
axis ['Ch5','Ch7','Ch10']  -> out.axes['ch'].data ['Ch5' 'Ch7']   correct
axis ['Ch9','Ch8','Ch1']   -> out.axes['ch'].data ['Ch5' 'Ch7']   should be ['Ch9' 'Ch8']

Same class of error as the one in your description — right-looking label, wrong channel's samples — just reached through the positional path.

Two ways out. Either drop the shortcut entirely (any message carrying a coordinate axis is value-sensitive, because the output axis is), or keep it and stop caching new_axis, recomputing it per message outside _reset_state. I'd drop it: per the benchmark it saves 0.16 µs of split(",") and costs a live bug. If you keep it, it needs to be conditional on the axis having no coordinate data at all, not on the selection being positional.

[cboulay]

A few sharp edges here, all of which coord_value_fingerprint (a util/channels.py helper I've written but not yet pushed) handles — I'd suggest return hash(key + coord_value_fingerprint(message, axis, fields)) and deleting this branch:

Object-dtype axes silently reset every message. An object array's buffer is pointers, so two equal label arrays built from distinct string objects checksum differently:

a = np.array([f"ch{i}" for i in range(8)], dtype=object)
b = np.array(["".join(("ch", str(i))) for i in range(8)], dtype=object)
hash(a.tobytes()) == hash(b.tobytes())   # False

That's a silent perf cliff rather than a wrong answer, but it inverts test_slicer_unchanged_axis_does_not_reset_state for anyone whose source hands over an object array. The helper widens with .astype("U") first.

It hashes fields the selection never reads. With field="bank" on a ChannelMap axis, jitter in x/y — a source recomputing float electrode positions — forces a pointless re-resolve. This is the real argument for narrowing to the consulted field, more than speed: coord_value_fingerprint(message, axis, ("bank",)) tracks bank and ignores the rest.

Note the fields Slicer consults are exactly what _axis_labels decides: (field,) when settings.field is set, ("label",) for a structured axis carrying one, and None (whole array) for a plain axis. That logic wants to be shared with _axis_labels rather than re-derived, so the hash and the resolution can't drift apart.

np.asarray(data) assumes numpy. Fine today, but it raises on a cupy-backed coordinate axis rather than degrading. The helper goes through np.ascontiguousarray, which also covers the strided case (a sliced coordinate array has no C-contiguous buffer).

Minor: tobytes() alone carries neither dtype nor shape. n_ch is already in the key so it barely matters, but they're nearly free to include — with one catch worth knowing, since it bit me: str(dtype) on an 8-field structured dtype costs 10 µs, ten times the checksum it annotates. Store the np.dtype object instead (0.03 µs, hashable, compares by value).

[cboulay]

This re-splits a string on every message to answer a question fixed at construction — settings is frozen, so _resolves_by_position() can never change for a given transformer.

It's only 0.16 µs, but group_spec_fingerprint's docstring frets about the function call itself being a measurable share of cost at this rate, so a split() plus a generator per message is out of keeping. functools.cached_property, or resolve it once in _reset_state.

(Also, narrowly: field is not None returning False is stricter than needed — with field="bank" and selection "3:4", parse_slice takes the two-part slice path and never consults the labels. Moot if the shortcut goes away.)

[cboulay]

This test passes on a broken implementation. It asserts on .data but never on .axes["ch"].data, which is where the positional path goes stale (see the comment on _hash_message). Adding

assert np.array_equal(out.axes["ch"].data, np.array(["Ch9", "Ch8"]))

turns it into a failing test for the bug.

The docstring's second claim — "hashing coordinate data no positional selection can consult is pure cost" — is the part that doesn't hold: _reset_state does consult it, to build new_axis.

Two more cases I'd want covered once the mechanism settles: an object-dtype label axis (should behave like test_slicer_unchanged_axis_does_not_reset_state, and currently doesn't), and a structured axis where an unread field changes while the selected field doesn't (should not reset).