Feat/time dependent config engine - #225
Open
viktorbeck98 wants to merge 9 commits into
Open
Conversation
Add a time_dependent config that cuts StabilityClassifier segments at equal-duration boundaries of the observed time span instead of equal-count boundaries of the change series. - StabilityClassifier.is_stable gains optional timestamps; boundaries via np.searchsorted on equal-duration cuts, count-mode fallback on missing/mismatched/non-finite timestamps or zero span (never raises) - SingleStabilityTracker(time_dependent=...) stores exact per-occurrence timestamps; state round-trips via msgpack, legacy snapshots load - timestamp threaded EventPersistency.ingest_event -> add_data -> add_value as optional trailing param; dataframe backends accept+ignore - EventStabilityTracker(time_dependent=...) forwards via closure factory (survives load(), mirroring expand_value) Default off is bit-for-bit unchanged. 17 new tests; full suite 550 pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
viktorbeck98
marked this pull request as ready for review
July 21, 2026 13:30
Merge left both sides' __init__ stacked instead of combined and a duplicate return in make_tracker. Merge the params (expand_value/ time_dependent + add_value_fn/detector_config) into one __init__, use self._accum in add_value so expand_value takes effect, and drop the dead return. Narrow timestamps for mypy without assert and consolidate test imports via the trackers re-export. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Unrelated to the time-dependent feature; the deleted section documents the mypy/flake8/bandit/vulture gates and the Python 3.12 requirement.
…lue_fn Segments a variable's change series by equal time spans instead of equal sample counts when timestamps are available, so a variable that churns in a burst and then settles is judged on when its values changed rather than on how many samples separated them. Reconciles the feature with development's add_value_fn hook, which a merge had collided with. A detector no longer replaces the tracker's add_value; the tracker calls it through _value_fn and keeps ownership of the two lists that must stay aligned. A detector that records nothing for a value (ValueRangeDetector on non-numeric input) therefore cannot desync timestamps from change_series. That collision was failing 35 tests. Drops expand_value, which 895a0d4 replaced with add_value_fn and a botched merge resurrected: nothing passed it, CharsetDetector already provides its semantics, and it was silently inert whenever add_value_fn was set. Config surface: time_dependent, timestamp_variable and timestamp_format on VariableDetectorConfig. The event time is read from logFormatVariables and parsed by the existing TimeFormatHandler; a missing or unparseable value warns once per detector and falls back to count segmentation, so a typo'd variable name is not mistaken for a working time-dependent run. The flag is merged into the tracker kwargs in VariableDetector.__init__ rather than in _stability_kwargs, so it also reaches NewValueDetector and NewValueComboDetector, and is preserved across the set_configuration reassignment that auto_config performs before training. StabilityClassifier now has one segment-boundary rule for both the RLE and plain-list paths; _compute_segment_means used np.array_split (remainder front-loaded) while _segment_boundaries floored. Equal-duration cuts fall back to count boundaries when they would leave a segment empty or when timestamps are not monotonic, since an empty segment yields a nan mean that passes every threshold unconditionally. Downstream: SingleTracker.add_value and EventDataStructure.add_data gain an optional trailing timestamp: float | None = None, so out-of-tree subclasses of those ABCs must accept it. Detector add_value(self, tracker, value) signatures are unaffected.
Replace the boolean VariableDetectorConfig.time_dependent with stability_segmentation: Literal["count", "time", "both"] (tracker kwarg, state key and event_data_kwargs key: "segmentation"). "count" is the historical behaviour and stays the default. "both" requires a variable to be stable under equal-count *and* equal-duration segmentation. Neither subsumes the other -- a variable that churns in a burst and then settles is count-UNSTABLE but time-STABLE, and one whose late churn is buried under a dense settled tail is the reverse -- so the conjunction is strictly stricter than either input. It is implemented entirely in SingleStabilityTracker._is_stable(), which calls StabilityClassifier twice over the same change series; the classifier stays mode-agnostic and is unmodified. Degradation is free: when timestamps are missing, misaligned or unusable, _aligned_timestamps() returns None, the time pass reuses the count boundaries and "both" collapses to plain "count" rather than to an unconditional pass. classify() now reports the segment means on the UNSTABLE branch too, not only on STABLE -- that is the verdict worth debugging, and in "both" mode the reason carries both mean vectors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…count An equal-duration cut that left a segment with no observations used to discard the time boundaries wholesale and re-cut the series by count. That guard existed because an empty segment produced a nan mean and `not nan >= thresh` is an unconditional pass -- a bursty variable could be called STABLE while churning. Score the empty segment 0.0 instead: nothing observed in a window means nothing changed in it. segment_means is now always finite, so the verdict no longer leans on nan comparison semantics, and time mode keeps its own boundaries in exactly the case the fallback used to swallow. The leniency the guard prevented is real and now reachable: a burst of churn followed by silence is time-STABLE with means [0.5, 0, 0, 0]. "both" is the answer to it -- the count pass keeps every segment populated and still calls that series UNSTABLE. Co-Authored-By: Claude Opus 5 <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.
Task
Description
Adds a
stability_segmentationmode to stability classification, so auto-configuration can judge a variable by wall-clock time rather than event count — or by both.VariableDetectorConfig.stability_segmentation: "count" | "time" | "both", default"count"(bit-for-bit identical to previous behavior).timestamp_variable/timestamp_formatname and parse the record's event time.StabilityClassifier.is_stable()takes an optionaltimestampslist and cuts its 4 threshold segments at equal-duration boundaries vianp.searchsorted. Silently falls back to equal-count on missing / mismatched / non-finite / out-of-order stamps or zero span — it never raises. A segment with no observations in it scores a mean of0.0."both"requires stability under both segmentations. Neither subsumes the other: a burst of churn then silence is count-UNSTABLE / time-STABLE, and late churn buried under a dense settled tail is the reverse. Implemented inSingleStabilityTracker._is_stable()by running the classifier twice over the same change series; the classifier itself stays mode-agnostic and is unmodified by that commit.timestamp: float | None = Noneis threaded throughEventPersistency.ingest_event→EventDataStructure.add_data→ trackeradd_value; dataframe backends accept and ignore it.EventStabilityTracker(segmentation=...)forwards the mode.set_configuration; legacy snapshots still load and default to"count".docs/detectors.md, including every fallback path.0.0,timeon its own is lenient towards a burst of churn followed by silence — the quiet quarters are empty and pass. Usebothwhere that matters; the count pass keeps every segment populated and still calls such a series UNSTABLE.SingleTracker.add_value/EventDataStructure.add_datasignatures gained an optional trailingtimestampparameter — out-of-tree subclasses must accept it.Design/formulation background lives in the
configuration_engine2.0research repo.How Has This Been Tested?
tests/test_persistency/test_time_dependent_stability.py: boundary computation and each fallback path (None/NaN/zero-span/length-mismatch/out-of-order), count-vs-time divergence in both directions, thebothconjunction, persistence round-trip, and end-to-end plumbing throughEventPersistency. Plus segmentation config-preservation cases intests/test_detectors/test_new_value_combo_detector.py.time_dependentflag, worth a re-run before merge.Checklist