EWMA/scaler: add opt-in reset_on_resume for passthrough gaps - #200
Open
cboulay wants to merge 1 commit into
Open
EWMA/scaler: add opt-in reset_on_resume for passthrough gaps#200cboulay wants to merge 1 commit into
cboulay wants to merge 1 commit into
Conversation
EWMATransformer and AdaptiveStandardScalerTransformer short-circuit passthrough before _hash_message, so switching passthrough back off resumes from a `zi` describing an exponentially-weighted window that ended when passthrough was switched on. The gap is invisible in the state -- a 10 ms blip and a 10 minute outage resume identically -- and for the scaler that means z-scoring post-gap data against pre-gap statistics. Add `reset_on_resume` to EWMASettings (inherited by AdaptiveStandardScalerSettings), defaulting to False so current behaviour is unchanged. True rebuilds from the first post-gap message instead, matching the position BinnedAggregateTransformer already takes. Two details worth noting: - The reset is driven from __call__/__acall__ rather than from update_settings, so it also holds when settings are pushed by assignment. Both flags stay in NONRESET_SETTINGS_FIELDS: a toggle with no messages in between leaves no gap and so should not reset. - The passthrough and empty-message short-circuits are now separate. An empty chunk passes no samples to the filter and so leaves no hole in zi's history; only passthrough does. Sharing one `or` would have made every empty chunk look like a gap. Refs #195
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.
Addresses #195, as an opt-in setting rather than a behaviour change.
The problem
EWMATransformerandAdaptiveStandardScalerTransformershort-circuit passthrough in__call__/__acall__, returning before_hash_message, soself._hashkeeps whatever value it had when passthrough was switched on. Switching passthrough back off therefore resumes from the state as it was before the gap.ziis an exponentially-weighted average over the recent past. During passthrough the filter sees none of the samples that go by, so on resume it describes a window that ended when passthrough began — and the gap is invisible in the state, so a 10 ms blip and a 10 minute outage resume identically. For the scaler that means z-scoring post-gap data against pre-gap mean and variance.The change
reset_on_resume: bool = FalseonEWMASettings, inherited byAdaptiveStandardScalerSettings.Default False preserves current behaviour — the three tests pinning seamless resume are untouched, and the existing
passthroughdocstring promise still holds. A short blip arguably should resume seamlessly, and resetting discards an estimate that may have taken manytime_constants to converge.True rebuilds from the first post-gap message instead, matching the position
BinnedAggregateTransformeralready takes deliberately for the same flag. With the existing bias correction the first post-gap output is exactly the first sample, and the estimate re-converges overtime_constant. For the scaler,_reset_staterebuilds both child EWMAs, so a parent reset propagates without extra work.Two details
The reset is driven from
__call__/__acall__, notupdate_settings. So it also holds when settings are pushed by assignment (proc.settings = replace(...)) rather than throughupdate_settings. Bothpassthroughandreset_on_resumestay inNONRESET_SETTINGS_FIELDS: a toggle with no messages in between leaves no gap and so should not reset.The passthrough and empty-message short-circuits are now separate. They were one
or:An empty chunk carries no samples past the filter and so leaves no hole in
zi's history; only passthrough does. Sharing one condition would have made every empty chunk look like a gap. Both are now routed through a small_skip()helper that documents the distinction, with a test pinning that an empty chunk — including one arriving during passthrough — leaves_hashuntouched.Tests
New, all exercising
reset_on_resume=True(the default path is covered by the existing tests):test_ewma_reset_on_resume_discards_state— output after the gap matches a never-used transformer, not one carrying the pre-gap estimate. Also asserts the state is left alone during the gap; only the hash is invalidated, so the rebuild happens on the next real message rather than eagerly.test_ewma_reset_on_resume_via_update_settings— toggling queues no reset by itself; the first passthrough message does.test_ewma_reset_on_resume_ignores_empty_messages— the case the split exists to protect.TestAdaptiveStandardScalerPassthrough::test_reset_on_resume_discards_state— both child EWMAs are replaced on resume.Full suite: 3693 passed, 5 skipped. Ruff clean.