Follow chunk_dim rather than assuming "time" - #11
Merged
Merged
Conversation
`ShMemCircBuffSettings.axis` defaulted to `"time"`, and the ring is a history
of the stream, so it has to be the dimension messages accumulate along. That is
`time` on a raw signal and `win` downstream of a windowing stage, and only the
producer reliably knows which.
Nothing rejected the old default on a windowed stream, because `time` *is*
present in a `(win, time, ch)` message. It just buffered the wrong thing:
buffered axis = 'time'
n_win=4: frame_shape=(4, 3) frames written=10 srate=100
n_win=7: frame_shape=(7, 3) frames written=10 srate=100
buffered axis = 'win'
n_win=4: frame_shape=(10, 3) frames written=4 srate=10
n_win=7: frame_shape=(10, 3) frames written=7 srate=10
The window *count* lands inside `frame_shape`, so the buffer is reallocated
whenever it jitters, and the rate published to the viewer is the within-window
rate -- a 10x error in its time base for a 10-sample window.
`axis` now defaults to None, meaning "follow the message". An explicit setting
still wins, so an operator can drive a producer that declares nothing; without
either, the old `"time"` fallback stands. The buffered dimension is resolved per
message and held in state, and the buffer is torn down if it changes -- a
windowing stage inserted upstream leaves the ring describing the old layout.
The blob gains `chunk_dim`, recorded separately from `buffered_axis` so a
consumer can tell the source's declaration from an operator's override, and the
mirror exposes both. Adding a key deliberately does not bump
`AUX_FORMAT_VERSION`: a reader that predates it ignores what it does not know
and a newer reader defaults it, so a mixed-version link -- the pairing this
plain-dict format exists to support -- keeps working. Bumping would break
exactly that. There is a test for a blob written without the key.
`_axis_equal` asks for `CoordinateAxis.fingerprint` before reading bytes. It is
on the publisher's per-message path, and its identity shortcut misses precisely
when a producer rebuilds its axes -- where the cached digest, which every ezmsg
source now primes, turns an O(bytes) comparison into a comparison of two
tuples. Absent on older ezmsg and None for an undigestable dtype, so it stays a
pure fast path. Its docstring is also corrected: the `CoordinateAxis.__eq__`
MRO bug it describes was fixed in ezmsg 3.10, but the explicit comparison
stays, because the two halves of a link need not share a version.
The same wrong assumption was in the viewer and sigmon plot paths, where a
sweep keyed on `time` draws each window's interior along the x-axis and treats
the windows as channels. Both now go through `describe.stream_axis`, which
prefers the declaration and falls back to the old guess. A declaration naming
a dimension the message no longer has is ignored rather than trusted.
Requires ezmsg 3.10.0b2 for both fields.
72 passed, up from 52. Mutation-checked: ignoring `chunk_dim` in the sink fails
1, dropping it from the blob fails 3, making the fingerprint path answer
unconditionally fails 3, and both failure modes of `stream_axis` fail 1 each.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AxisArray.chunk_dim(ezmsg 3.10) names the dimension messages accumulate along. Three places here were guessing"time"instead, and one of the guesses is wrong in a way that reaches the display.The shmem sink buffers the wrong dimension
ShMemCircBuffSettings.axisdefaulted to"time". The ring is a history of the stream, so it has to be the dimension messages accumulate along —timeon a raw signal,windownstream of a windowing stage.Nothing rejected the old default on a windowed stream, because
timeis present in a(win, time, ch)message. It just buffered the wrong thing:Two consequences:
frame_shape, so the buffer is reallocated whenever it jitters — which for a windowing stage is most messages;axisnow defaults toNone, meaning "follow the message". An explicit setting still wins so an operator can drive a producer that declares nothing, and without either the old"time"fallback stands — so no existing configuration changes behaviour. The resolved dimension is held in state and the buffer is torn down if it changes, since a windowing stage inserted upstream leaves the ring describing the old layout.The viewer and sigmon sweeps had the same assumption
A sweep keyed on
"time"draws each window's interior along the x-axis and treats the windows as channels, reading an offset that doesn't advance with the stream. Both now go through a newdescribe.stream_axis, which prefers the declaration and falls back to the old guess. A declaration naming a dimension the message no longer has is ignored rather than trusted.chunk_dimon the wireThe aux blob carries it, recorded separately from
buffered_axisso a consumer can tell the source's declaration from an operator's override. The mirror exposes both.Adding a key deliberately does not bump
AUX_FORMAT_VERSION. A reader that predates it ignores what it doesn't know; a newer reader defaults it when an older writer omits it. Both directions keep working, which is the mixed-version pairing this plain-dict format exists to support — bumping would break exactly that. The module docstring now states the policy, and there's a test for a blob written before the key existed._axis_equalasks for the fingerprint firstIt's on the publisher's per-message path. Its identity shortcut misses precisely when a producer rebuilds its axes — and that's where
CoordinateAxis.fingerprint, which every ezmsg source now primes at construction, turns an O(bytes)array_equalinto a comparison of two tuples. Absent on older ezmsg andNonefor an undigestable dtype, so it stays a pure fast path: it can make the check cheaper, never wrong.Its docstring is also corrected. The
CoordinateAxis.__eq__MRO bug it describes was fixed in ezmsg 3.10 — but the explicit field-by-field comparison stays, because the two halves of a shmem link need not share an ezmsg version and a writer on 3.9 is still one this has to be correct for.Testing
91 passed, up from 52 before this branch (72 mine, plus 19 from #10 which landed while I was working). Mutation-checked:
chunk_dimchunk_dimstream_axisignores the declarationstream_axistrusts a stale declarationDependency
ezmsg>=3.10.0b2, a pre-release pin until 3.10.0 ships. Pinned directly rather than left transitive: uv only enables pre-releases for a package named with a pre-release marker in this file.🤖 Generated with Claude Code