Resolve the operating dimension from chunk_dim across the processors - #28
Merged
Conversation
Five distinct cases, not one:
* ssr.py names a *channel* axis, so it skips the chunk dim rather than
following it (resolve_feature_dim). dims[-1] is the chunk dim on a
(ch, time) stream, which would have regressed across time. The setting
stays: naming a channel axis is a real choice.
* flatten.py mirrors ezmsg-sigproc's Flatten -- the setting stays, only the
default changes, because Flatten holds no data between messages.
* slda.py caches an output template keyed to the dimension its samples
accumulate along; `axis` deprecated, removed in 2.0.
* adaptive_decomp/incremental_decomp: `axis="!time"` meant "iterate over
time, decompose the rest", and the non-! branch guessed the iteration axis
with a hand-rolled `"win" if "win" in dims else "time"`. Both are what
chunk_dim answers. `axis=None` is now the default and does what "!time"
spelled; the "!" spelling is deprecated. Naming a target axis is untouched.
This also settles incremental_decomp's `TODO: This iter_axis is likely
incorrect` -- there is no message to resolve from in _initialize_processors,
so it defers to Window, which resolves chunk_dim itself.
* sample_adapt_regressor.py stops hardcoding axis="time" for its Window, and
deprecates `resample_axis`, forwarding it under suppress_axis_deprecation so
ezmsg-sigproc does not warn about a class the user never touched.
sgd.py flattened everything but `dims[0]` into the feature vector, which
assumed the streaming axis came first. It now resolves and moves that axis to
the front. It declares STREAMING_DIMS = ("win", "time") because it is fed
windows: a producer that declares no chunk_dim is accumulating along `win`
here, and the base default would have folded the windows into the features.
Fixes five tests that were already failing on dev. IncrementalDecomp unbundles
its training windows with iter_over_axis("win"), which slices the dimension
away while chunk_dim still named it -- and an AxisArray rejects a chunk_dim
that is not among its dims. The declaration is dropped before the slice.
ezmsg-sigproc 3.8.0 shipped its own copy of the deprecation machinery before it moved to ezmsg-baseproc, so its warnings check a ContextVar that baseproc's suppress_axis_deprecation() does not set. Import the context manager from ezmsg.sigproc, which works against 3.8.0 and against later versions that re-export baseproc's.
ezmsg-sigproc 3.8.1 shares baseproc's deprecation mechanism, so there is a single ContextVar again and the import that works is the obvious one. Pinned to >=3.8.1 accordingly.
Slicing "win" away leaves each sub-message no longer a chunk along it. Clearing the declaration was enough to stop AxisArray rejecting them, but it left the decomp resolving the iteration axis from STREAMING_DIMS when the message can simply say it: successive windows advance along the within-window axis, which is the one the offset fix-up right below re-anchors. Newer ezmsg clears chunk_dim itself when iter_over_axis consumes the dimension, so the explicit clear before the loop is only there to keep this working on versions that do not.
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.
Requires ezmsg-baseproc 1.13.0 and ezmsg-sigproc 3.8.0 (both released).
Five distinct cases, not one
The
axissettings here don't all mean the same thing, so they don't all get the same treatment:ssr.pyresolve_feature_dim— setting staysflatten.pyslda.pyadaptive_decomp/incremental_decomp"!X"= iterate over X!spelling deprecatedsample_adapt_regressor.pyresample_axisssr.py—dims[-1]is the chunk dim on a(ch, time)stream, so the old default would have regressed across time. Naming a channel axis is a real choice, so the setting stays.flatten.py— mirrors sigproc'sFlatten: it holds no data between messages, so only the default changes.slda.py— caches an output template keyed to the dimension its samples accumulate along.axis="!time"meant "iterate over time, decompose the rest", and the non-!branch guessed the iteration axis with a hand-rolled"win" if "win" in dims else "time". Both are exactly whatchunk_dimanswers.axis=Noneis the new default and does what"!time"spelled; naming a target axis (axis="ch") is untouched and stays silent. This also settlesincremental_decomp's# TODO: This iter_axis is likely incorrect— there's no message to resolve from inside_initialize_processors, so it defers toWindow, which resolveschunk_dimitself.sample_adapt_regressor.py— stops hardcodingaxis="time"for itsWindow, and forwardsresample_axisundersuppress_axis_deprecationso sigproc doesn't warn about a class the user never touched.sgd.pyhad a positional assumption in the mathIt flattened everything but
dims[0]into the feature vector — assuming the streaming axis came first. It now resolves that axis and moves it to the front.It also declares
STREAMING_DIMS = ("win", "time"), because it is fed windows: a producer that declares nochunk_dimis accumulating alongwinhere, and the base("time",)default would fold the windows into the feature vector. I found this the hard way — my first attempt calledresolve_chunk_dim(message)without threading the class'sSTREAMING_DIMSthrough, so the override silently did nothing and the feature count changed from 6 to 4.Fixes five tests that were already red on
devIncrementalDecompunbundles its training windows withiter_over_axis("win"), which slices that dimension away whilechunk_dimstill names it — and anAxisArrayrejects achunk_dimthat isn't among itsdims. The declaration is now dropped before the slice.I verified these were pre-existing by running unmodified
devagainst sigproc 3.7.0 as well as 3.8.0 — 5 failures either way, so neither this PR nor the sigproc upgrade caused them.The one test change is an assertion that pinned the hardcode this PR removes (
win.settings.axis == "time"→is None).Testing
369 passed, 24 skipped — up from 362 passed / 7 failed. Deprecation behaviour checked by hand across all seven cases:
axis=Noneandaxis="ch"silent,"!time"/slda axis/resample_axiseach warn once naming our class.