Delegate EventRate binning to shared BinSchedule; add fractional=False mode#33
Merged
Conversation
EventRate's bin grid (samples-per-bin, fractional carry, global bin index,
output gain/offset) was computed inline and duplicated the same arithmetic
ezmsg-sigproc's dense binner uses. Both now consume the shared
ezmsg.sigproc.util.binning.BinSchedule, so they land on an identical grid by
shared code rather than by coincidence.
- BinnedKernelActivationState drops bin_accumulator in favor of a BinSchedule
(fractional, since EventRate bins always track the nominal bin_duration).
- _process_events and _process_dense_count_sum now get n_bins, chunk-local bin
ends, output gain and output offset from BinSchedule.advance(); the per-bin
reduction and activation-state logic are unchanged.
- New cross-package test asserts EventRate and BinnedAggregate(SUM) produce the
same output time axis and bin counts at fs in {1000, 30012, 30030} under
adversarial chunking -- the alignment the shared primitive guarantees.
Requires an ezmsg-sigproc release that ships BinSchedule; the dependency pin
will need bumping before this can merge.
BinSchedule already carries the fractional flag, so exposing a sample-locked mode on EventRate is just plumbing plus using the schedule's actual gain. - EventRateSettings/BinnedKernelActivationSettings gain a `fractional` field (default True; threaded through Rate). False bins on Window's int(bin_duration*fs) grid so a sample-locked SBP branch and the spike-rate branch share one grid. - Both output paths now label the axis with step.output_gain (the schedule's actual seconds-per-bin) instead of the nominal bin_duration, and rate_normalize divides by that same actual duration. In fractional mode output_gain == bin_duration, so default behavior is unchanged (full suite still green). - Cross-package test asserts EventRate(fractional=False) is sample-locked to Window's gain and shares the grid with BinnedAggregate(fractional=False).
binned.py still carried its own integer-locked event-counting binner, the last event binner not on the shared grid. Rewrite it as a thin wrapper over ezmsg.sigproc.binned_aggregate.BinnedAggregate (operation=SUM): densify sparse events to per-sample contributions, delegate all boundary/carry/axis math to the shared binner, and optionally rate-normalize. Ported from #32 (Kyle McGraw), adjusted for the enhanced sigproc shipped in 2.26.0: - fractional=False is documented and tested as int(bin_duration*fs) truncation (matching Window and BinSchedule), not round(); the sample-locked test now includes fs=30030 where truncation (600) and rounding (601) diverge. - Adds scale_by_value (weight events by stored value), a binned_event_aggregator factory, and tests for the off-nominal fractional grid, chunk invariance, count-vs-rate scaling, scale_by_value, and the sample-locked grid. This leaves Rate/EventRate on the kernel_activation path (event-optimized, O(n_events) sparse) from the companion commits; BinnedEventAggregator is now also on the shared grid, so no event binner remains on the old divergent one.
Drop references to consumer-pipeline concepts that public users of these packages won't recognize -- named "spike-band-power"/"spike-rate" branches, the "downstream Merge with no post-hoc reconciler" narrative, "real recordings", and the confusing "wall-clock" framing. Describe the behavior in terms of this package and the sigproc binner it delegates to (BinnedAggregate, Window, BinSchedule, Merge, ThresholdCrossing) instead. Comment-only.
The small functional-constructor helpers are a deprecated pattern; new modules shouldn't add them. BinnedEventAggregator(BinnedEventAggregatorSettings(...)) is the supported construction path. Unused — no callers.
Member
Author
Relationship to #32Both PRs put the event binners on sigproc's shared
This PR also carries over #32's Net: |
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
1 participant
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.
Summary
EventRate(viaBinnedKernelActivation) computed its bin grid — samples-per-bin, fractional carry, global bin index, output gain/offset — inline. That arithmetic was a duplicate of whatezmsg-sigproc's dense binner does, and the two only agreed by coincidence, which is what caused SBP-vs-SPK timestamp mismatches at off-nominal sample rates (e.g. ~30012 Hz) where a downstreamMerge/AlignAlongAxisnever lined up.This routes
BinnedKernelActivation's boundaries through the sharedezmsg.sigproc.util.binning.BinSchedule(shipped in ezmsg-sigproc v2.26.0), so the spike-rate branch and any sigproc binner land on an identical grid by shared code, not by luck.Changes
BinSchedule.BinnedKernelActivationStatedropsbin_accumulatorin favor of aBinSchedule(fractional by default — EventRate bins track the nominalbin_duration)._process_eventsand_process_dense_count_sumnow getn_bins, chunk-local bin ends, output gain and output offset fromBinSchedule.advance(); the per-bin reduction and activation-state logic are unchanged.fractional=False(sample-locked) mode.EventRateSettings/BinnedKernelActivationSettingsgain afractionalfield. WhenFalse, bins are sample-locked toint(bin_duration*fs)samples — matchingezmsg.sigproc.window.Window— so a sample-locked SBP branch and the spike-rate branch can share one grid that way too.rate_normalizenow use the schedule's actual seconds-per-bin (step.output_gain) instead of the nominalbin_duration. In fractional mode these are identical, so default behavior is byte-for-byte unchanged.ezmsg-sigproc>=2.26.0(shipsBinSchedule).Tests
tests/test_bin_schedule_crosspackage.py: assertsEventRateandBinnedAggregateproduce the same output time axis and bin counts under adversarial chunking at fs ∈ {1000, 30012, 30030} — for bothfractional=True(shared wall-clock grid) andfractional=False(shared sample-locked grid, matchingWindow's gain).fractional=Truepath is unchanged.Context
Companion to ezmsg-sigproc's
BinScheduleextraction (PR intogeneralize-binner, released in 2.26.0). Together they replace "agreement with EventRate by construction, untested" with one boundary primitive plus regression tests on both sides.