Adopt the ezmsg-sigproc 3.0 channel-grouping API - #21
Merged
Conversation
ezmsg-sigproc#212 collapses three channel-grouping spellings into one ChannelGroupSpec, and stops letting a caller-supplied grouping influence how the affine matmul is blocked -- block structure is now always read off the weight matrix itself. Track that here. Required by the upstream renames: - channel_clusters_from_field / validate_channel_clusters -> channel_groups_from_field / validate_channel_groups (we now use the higher-level resolve_channel_groups instead). - rereference_matrix(clusters=) -> groups=. - AffineTransformSettings.channel_clusters -> channel_groups, and min_cluster_size removed. Beyond the mechanical renames: - SelfSupervisedRegressionSettings.channel_clusters + cluster_by_field collapse into one channel_groups: ChannelGroupSpec, mirroring upstream. Explicit index groups, a metadata field name, a tuple of field names and callables are all accepted through the same setting; block_size remains the fallback when the spec is None or resolves to nothing. This deletes the hand-rolled precedence chain and the bespoke field-presence hashing in favour of resolve_channel_groups() and group_spec_fingerprint(). - Drop the channel_clusters=None workaround added in #20. It existed to stop a grouping finer than W's true blocks from corrupting the apply; upstream removed that failure mode entirely, so there is nothing left to work around. Neither affine is given a grouping now: LRR always supplies an explicit weight matrix, and grouping only builds kind/callable weights. - LRRSettings.min_cluster_size -> kernel ("auto"|"dense"|"blocks"), forwarded to the affine. The old setting tuned a merge threshold that no longer exists; kernel is the supported escape hatch from the new cost model, which upstream documents as mispredicting above ~2000 channels -- squarely LRR's regime. - Rename cluster -> group throughout (MIN_REREF_CLUSTER_SIZE -> MIN_REREF_GROUP_SIZE, _get_channel_clusters -> _get_channel_groups, state.resolved_clusters -> resolved_groups) so one vocabulary spans both packages. - _solve_weights: the "group is every channel, skip the scatter" shortcut now also requires the indices to be in order. A callable spec may return all n channels permuted, which still needs scattering back. Requires ezmsg-sigproc>=3.0.0. Inference speedup on this change (512 ch, 8x64 groups, median of 200): NumPy 3.2x at chunk=20 rising to 7.5x at chunk=300; Torch MPS ~3-4x across the range; MLX ~2x. Torch MPS at chunk=20 was over the 30 kHz real-time budget (816 us against 667) and is now at 217 us. partial_fit is unchanged, as it does not go through the affine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LRRUnit takes training on INPUT_SAMPLE and signal on a separate stream, so
several partial_fit calls routinely land before the first message is
processed. In that window the transform silently applied the *first* fit
forever:
p = LRRTransformer(LRRSettings(incremental=False))
p.partial_fit(msg(X1))
p.partial_fit(msg(X2)) # no message processed in between
p.send(msg(X2)) # -> X2 @ (I - W1), not (I - W2)
_on_weights_updated built the affine eagerly, passing I - W as
AffineTransformSettings.weights. The affine's _reset_state -- which does not
run until a message arrives -- rebuilds its state from those settings, so
every set_weights() call in between updated state that was about to be
discarded. Wrong output of correct shape and plausible magnitude, no warning.
Construction moves to _process, which builds from LRRState.effective, the
latest I - W. _on_weights_updated now only refreshes that cache and updates
an already-built affine in place. The in-place path is unchanged and still
the common case: once a message has been seen, set_weights() with the
default recalc_structure=False keeps the block layout and swaps the values.
Later re-resets cannot reintroduce the staleness. The affine's hash is
(key, n_channels) plus an empty group fingerprint, so anything that would
trigger its _reset_state trips LRR's own hash first, and that nulls the
affine and forces a rebuild from the cache.
TestRefitBeforeFirstMessage covers both paths: refit before the first
message (asserting the output is not the stale first fit) and refit after
the affine exists.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The repo pinned ruff to 0.16.2 in the lint dependency-group and in .pre-commit-config.yaml, but the Lint job in python-tests.yml called `uv tool run ruff`, which resolves ruff independently of the project and so honoured neither pin. CI would have silently tracked whatever ruff was newest at run time, and a future release could turn it red with no change to this repo. Use `uv run` instead, taking ruff from the locked lint group so pyproject.toml is the single source of truth. Two pre-existing violations are cleared so `pre-commit run --all-files` is green. Neither was failing CI, which only lints `src` and does not check formatting: - tests/unit/test_adaptive_linear_regressor.py: unsorted import block (I001) - src/ezmsg/learn/collection/sample_adapt_regressor.py: ruff format Both fixes are import ordering and line wrapping only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Tracks the ezmsg-sigproc 3.0.0 channel-grouping API (ezmsg-org/ezmsg-sigproc#212), fixes an unrelated correctness bug in LRR, and makes the lint job deterministic.
Requires
ezmsg-sigproc>=3.0.0, now released.These affect anyone constructing
LRRSettings/SelfSupervisedRegressionSettings. There is no deprecation period — upstream removed the settings these forwarded to, so the old spellings could not keep working.1.
channel_clusters+cluster_by_field→ onechannel_groupsTwo settings that did the same job through different spellings collapse into a single
ChannelGroupSpec, mirroring upstream. All of these are now the same setting:channel_clusters=[[0, 1], [2, 3]]channel_groups=[[0, 1], [2, 3]]cluster_by_field="bank"channel_groups="bank"channel_groups=("array", "bank")— group by the field tuplechannel_groups=fn(message, axis)— callableblock_sizeis unchanged and remains the fallback whenchannel_groupsisNoneor resolves to nothing (e.g. a field spec the incoming axis doesn't carry).Precedence changed. Previously explicit
channel_clusterssilently won overcluster_by_fieldwhen both were set. With one setting there is nothing to arbitrate, so that rule is gone along with the test that pinned it. If you set both before, keep the one you actually wanted.2.
min_cluster_size→kernelmin_cluster_sizetuned a block-merge threshold that no longer exists — upstream now chooses the matmul kernel from a cost model over the weights. Replaced by a passthrough:min_cluster_size=1(the old "just use blocks" idiom) becomeskernel="blocks".min_cluster_size > n_channels(the old "force dense" idiom) becomeskernel="dense".This exists because upstream documents the cost model as mispredicting above ~2000 channels, picking up to ~40% off the best blocking — squarely LRR's regime.
kernelis the supported escape hatch; without it that tuning would be unreachable from this package.3. cluster → group rename
So that one vocabulary spans both packages:
MIN_REREF_CLUSTER_SIZEMIN_REREF_GROUP_SIZEstate.resolved_clustersstate.resolved_groups_get_channel_clusters()_get_channel_groups()_validate_clusters()_validate_groups()MIN_REREF_CLUSTER_SIZEis the only public one. Noteresolved_groupsnow holdsnp.ndarraygroups rather thanlist[int]— inherited from upstream's resolver.Migration at a glance
Upstream renames tracked
channel_clusters_from_field/validate_channel_clusters→channel_groups_from_field/validate_channel_groups. We now call the higher-levelresolve_channel_groups()instead, which handles every spec form and validates in-range and pairwise disjoint — overlapping groups used to pass silently here.rereference_matrix(clusters=)→groups=AffineTransformSettings.channel_clusters→channel_groups,min_cluster_sizeremovedset_weights(recalc_clusters=)→recalc_structure=. No call-site change:LRRTransformerpasses weights positionally and relies on the default, which is still what we want — refitting changes weight values, not the sparsity pattern.Dropped the
channel_clusters=Noneworkaround#20 pinned
channel_clusters=Noneon the internal affine so a grouping finer thanW's true blocks couldn't corrupt the apply. Upstream removed that failure mode outright — block structure is always read off the weight matrix now — so the workaround is deleted. Neither affine gets a grouping at all: LRR always supplies an explicit weight matrix, and grouping only builds kind/callable weights.The regression test from #20 stays, re-pointed at the guarantee from this side (
TestApplyFollowsWeightBlocks).Independent bug fix: stale weights when refit before the first message
Separate commit (
a643df8); not caused by the migration.LRRUnittakes training onINPUT_SAMPLEand signal on a separate stream, so severalpartial_fitcalls routinely land before the first message is processed. In that window the transform silently applied the first fit forever:_on_weights_updatedbuilt the affine eagerly, passingI - WasAffineTransformSettings.weights. The affine's_reset_state— which doesn't run until a message arrives — rebuilds state from those settings, so everyset_weights()in between updated state that was about to be discarded. Wrong output of correct shape and plausible magnitude, no warning.Construction moves to
_process, building fromLRRState.effective(the latestI - W). The in-placeset_weightspath is unchanged and still the common case. Covered byTestRefitBeforeFirstMessageon both paths.Also here
_solve_weights: the "group is every channel, skip the scatter" shortcut now also requires the indices to be in order. A callable spec may return allnchannels permuted, which still needs scattering back.527feb0): the Lint job calleduv tool run ruff, which resolves ruff independently and honoured neither thelintdependency-group pin nor.pre-commit-config.yaml. CI silently tracked whatever ruff was newest, so a future release could turn it red with no change here. Switched touv run ruff. Also cleared two pre-existing violations (an unsorted import block intests/, oneruff formatdiff insrc/) sopre-commit run --all-filesis green; neither was failing CI, which only lintssrcand doesn't check formatting.Performance
Inference speedup from the upstream kernel work, measured here at 512 channels in 8×64 groups, median of 200 iterations after 20 warmup, before = ezmsg-learn@dev + sigproc 2.34.0:
Torch MPS at chunk=20 was over the 30 kHz real-time budget (816 µs against 667) and is now at 217 µs.
partial_fitis unchanged — it doesn't go through the affine.Testing
Against released sigproc 3.0.0: 339 passed / 2 skipped (
tests/unit tests/dim_reduce), 6 passed (tests/integration), lint exit 0,pre-commit run --all-filesgreen.🤖 Generated with Claude Code