diff --git a/.claude/skills/choose-detector/SKILL.md b/.claude/skills/choose-detector/SKILL.md index 2dafbae..cc1f351 100644 --- a/.claude/skills/choose-detector/SKILL.md +++ b/.claude/skills/choose-detector/SKILL.md @@ -3,7 +3,8 @@ name: choose-detector description: | Help the user pick a drift detector and tune its settings for an apeiron run. Use when the user asks which detector to use, how to configure drift detection, - what ADWIN/KSWIN/PageHinkley/threshold values to set, or wants a ready-to-use + what ADWIN/KSWIN/PageHinkley/threshold values to set, whether to combine + detectors into an ensemble and which voting rule to use, or wants a ready-to-use [drift_detection] config block. Asks a few questions about the monitored metric and drift shape, recommends a detector, writes a filled-in TOML block, and validates that it loads. Does NOT run a full experiment — for that use @@ -31,19 +32,27 @@ it rather than restating numbers that may drift. user can paste into their config. ## Ground truth to respect (do not recommend around it) -Only three detectors are plug-and-play in the current `ContinuousMonitor` flow, -because `_check_drift()` calls `detector.update(agg_metric)` with a single -aggregated scalar: -- `ADWINDetector`, `KSWINDetector`, `PageHinkleyDetector`. - -The rest are **not** drop-in and must not be recommended as the default: -- `ModelPerformanceDetector` needs reference data + batch DataFrames (not passed by the monitor). -- `EvalDetector` (`ModelEvalDetector`) needs extra `update(...)` kwargs the monitor doesn't send. -- `EnsembleDetector` raises `NotImplementedError` in `load_drift_detector`. +`ContinuousMonitor._check_drift()` calls `detector.update(agg_metric)` with a +single aggregated scalar and no kwargs. Anything that needs more than that scalar +is not drop-in. + +Plug-and-play: +- `ADWINDetector`, `KSWINDetector`, `PageHinkleyDetector` — take the scalar directly. +- `EnsembleDetector` — its `update(value, **kwargs)` forwards the scalar to each + sub-detector, so it is drop-in **provided every name in `ensemble_detectors` is + one of the three scalar detectors above**. Listing a non-drop-in detector as a + sub-detector pushes the failure into the ensemble. + +**Not** drop-in; do not recommend as the default: +- `ModelPerformanceDetector` needs reference data + batch DataFrames (not passed by + the monitor); `update()` raises `ValueError` unless `set_reference()` ran first. +- `EvalDetector` (`ModelEvalDetector`) needs extra `update(...)` kwargs + (`modelHarness`, `reference_validation_metrics`, `higher_is_better`) the monitor + doesn't send. Confirm this is still true before relying on it: ```bash -sed -n '1,80p' src/apeiron/drift_detection/load_drift_detector.py +sed -n '1,100p' src/apeiron/drift_detection/load_drift_detector.py ``` If a user specifically wants one of the non-wired detectors, be honest that it requires extra wiring and point them at the integration notes in the doc. @@ -60,7 +69,8 @@ Ask the user (batch these with AskUserQuestion): - gradual/slow drift - distribution or variance change with little mean movement - **Sensitivity vs. false alarms**: react early and tolerate some false alarms, - or only fire on clear, sustained drift? + or only fire on clear, sustained drift? (A strong preference at either extreme, + or "I expect more than one kind of drift", is the cue to consider an ensemble.) - **Cadence**: roughly how many `update()` calls (i.e. `detection_interval`-sized checks) happen before they'd want a first detection, and how many batches per check. This sets warm-up expectations. @@ -74,6 +84,16 @@ Map answers to a detector: - distribution / variance / shape change without mean movement → **KSWIN** - abrupt mean shift, want fast + cheap detection → **PageHinkley** - gradual, mixed, or "not sure / general default" → **ADWIN** +- more than one drift shape expected, or an explicit sensitivity/false-alarm + preference a single detector can't express → **Ensemble** over two or three of + the above (see the voting rules in step 3) + +Prefer a single detector when one clearly fits — the ensemble costs an update on +every sub-detector per check and makes tuning harder to reason about, since each +sub-detector is still driven by its own hyperparameters in the same config block. +Reach for it when the shapes genuinely differ (e.g. PageHinkley for abrupt jumps +plus KSWIN for variance changes) or when the user wants a deliberate +sensitivity/conservatism bias they can state as a voting rule. State the recommendation and the one-line reason. If it's a close call, name the runner-up and the tradeoff. @@ -92,6 +112,21 @@ metric scale and the sensitivity preference: rarely fire, so start much smaller (order `1–10`) and tune; for larger-magnitude losses, larger thresholds are appropriate. `ph_delta` is the slack (min change treated as real); `ph_min_instances` is warm-up. +- **Ensemble** — `ensemble_detectors` is the list of sub-detector names; each is + built from this same `[drift_detection]` block, so a detector type can appear at + most once and still needs its own hyperparameters set here. An empty list, a + nested `"EnsembleDetector"`, or an unknown voting name raises `ValueError` at + load. `ensemble_voting` sets the bias: + - `any` (alias `or`) — fires when any sub-detector fires. Most sensitive; use + when a missed drift costs more than a needless CL dispatch. + - `majority` (default) — strictly more than half. Balanced; needs 3+ detectors + to mean anything (with 2 it behaves like `unanimous`). + - `unanimous` (aliases `all`, `and`) — every detector must fire. Most + conservative; suppresses small/noisy changes at the cost of latency. + Note `drift_score` is the mean of sub-detector scores and the regime is a + plurality vote, both independent of the voting rule — so the + `adwin_minor_threshold` / `adwin_moderate_threshold` regime split gets diluted + by sub-detectors that report a score of 0. Set `detection_interval`, `aggregation` (`mean`/`median`/`last`), `metric_index`, and `max_stream_updates` from the cadence answers. Explain any value that @@ -112,6 +147,24 @@ adwin_delta = 0.002 adwin_minor_threshold = 0.3 adwin_moderate_threshold = 0.6 ``` +For an ensemble, list the sub-detectors and keep each one's hyperparameters in the +same block: +```toml +[drift_detection] +detector_name = "EnsembleDetector" +ensemble_detectors = ["ADWINDetector", "PageHinkleyDetector"] +ensemble_voting = "unanimous" +detection_interval = 10 +aggregation = "mean" +metric_index = 0 +reset_after_learning = false +max_stream_updates = 20 + +# Sub-detectors read their usual hyperparameters from this same block +adwin_delta = 0.002 +ph_threshold = 30 +ph_delta = 0.5 +``` If `$1` was given, patch that file's `[drift_detection]` section (Edit); keep the keys that don't belong to the chosen detector untouched or drop the unused detector-specific keys, matching the existing file style. @@ -127,9 +180,14 @@ from apeiron.drift_detection.load_drift_detector import load_drift_detector cfg = build_config(['--config', '']) d = load_drift_detector(cfg) print('OK:', type(d).__name__) +print(getattr(d, 'voting', ''), [type(s).__name__ for s in getattr(d, 'detectors', [])]) print(cfg.drift_detection) " ``` +For an ensemble this is worth more than a syntax check: it is where an empty +`ensemble_detectors`, a nested `EnsembleDetector`, an unknown voting name, or an +unknown sub-detector name surfaces as a `ValueError` instead of at run time. The +second print confirms the resolved voting rule and that every sub-detector built. (`PYTHONPATH=src` is required so `import apeiron` resolves — the package lives under `src/apeiron` and `import apeiron` fails without it.) For a standalone block (no `$1`), write it to a temp file first and validate that @@ -145,5 +203,9 @@ detection behavior, hand off to `explore-examples` or `custom-experiment`. ## Notes - Quick way to A/B a detector on a shipped example without editing files: `poetry run python -m src.main --config examples/mnist/mnist.toml --set drift_detection.detector_name=PageHinkleyDetector --set drift_detection.ph_threshold=5` -- Keep `docs/drift_detectors.md` as the single source of truth; if you find this - skill and the doc disagree, fix the doc and follow it. +- `--set` values go through `json.loads`, so a list needs JSON syntax and shell + quoting: `--set 'drift_detection.ensemble_detectors=["ADWINDetector","KSWINDetector"]'` +- Precedence when sources disagree: the code in + `src/apeiron/drift_detection/` wins, then `docs/drift_detectors.md`, then this + skill. Fix whichever is stale rather than working around it — this file has + been wrong about detector wiring before. diff --git a/.codex/skills/choose-detector/SKILL.md b/.codex/skills/choose-detector/SKILL.md index dc871db..aa39e98 100644 --- a/.codex/skills/choose-detector/SKILL.md +++ b/.codex/skills/choose-detector/SKILL.md @@ -1,6 +1,6 @@ --- name: choose-detector -description: Help the user pick a drift detector and tune its settings for an apeiron run. Use when the user asks which detector to use, how to configure drift detection, what ADWIN/KSWIN/PageHinkley/threshold values to set, or wants a ready-to-use [drift_detection] config block. Asks a few questions about the monitored metric and drift shape, recommends a detector, writes a filled-in TOML block, and validates that it loads. Does not run a full experiment; for that use explore-examples or custom-experiment. +description: Help the user pick a drift detector and tune its settings for an apeiron run. Use when the user asks which detector to use, how to configure drift detection, what ADWIN/KSWIN/PageHinkley/threshold values to set, whether to combine detectors into an ensemble and which voting rule to use, or wants a ready-to-use [drift_detection] config block. Asks a few questions about the monitored metric and drift shape, recommends a detector, writes a filled-in TOML block, and validates that it loads. Does not run a full experiment; for that use explore-examples or custom-experiment. metadata: short-description: Recommend and configure a drift detector --- @@ -18,22 +18,29 @@ first and stay consistent with it rather than restating values that may change. ## Ground Truth -Only three detectors are drop-in for the current `ContinuousMonitor` flow, because -`_check_drift()` calls `detector.update(agg_metric)` with a single aggregated -scalar: +`ContinuousMonitor._check_drift()` calls `detector.update(agg_metric)` with a +single aggregated scalar and no kwargs. Anything needing more than that scalar is +not drop-in. -- `ADWINDetector`, `KSWINDetector`, `PageHinkleyDetector`. +Drop-in: -The others are not drop-in and must not be offered as defaults: +- `ADWINDetector`, `KSWINDetector`, `PageHinkleyDetector` take the scalar directly. +- `EnsembleDetector` forwards the scalar to each sub-detector via + `update(value, **kwargs)`, so it is drop-in provided every name in + `ensemble_detectors` is one of the three scalar detectors above. Naming a + non-drop-in detector as a sub-detector just moves the failure into the ensemble. -- `ModelPerformanceDetector` needs reference data and batch DataFrames. -- `EvalDetector` (`ModelEvalDetector`) needs extra `update(...)` kwargs the monitor does not send. -- `EnsembleDetector` raises `NotImplementedError` in the loader. +Not drop-in, and must not be offered as defaults: + +- `ModelPerformanceDetector` needs reference data and batch DataFrames; `update()` + raises `ValueError` unless `set_reference()` ran first. +- `EvalDetector` (`ModelEvalDetector`) needs extra `update(...)` kwargs the monitor + does not send (`modelHarness`, `reference_validation_metrics`, `higher_is_better`). Confirm before relying on this: ```bash -sed -n '1,80p' src/apeiron/drift_detection/load_drift_detector.py +sed -n '1,100p' src/apeiron/drift_detection/load_drift_detector.py ``` ## Procedure @@ -44,7 +51,7 @@ Ask the user: - Which metric feeds the detector, and its scale (bounded like accuracy/error in `[0, 1]`, or unbounded like a loss)? - Expected drift shape: abrupt mean jumps, gradual drift, or distribution/variance change with little mean movement. -- Sensitivity preference: react early and tolerate false alarms, or fire only on clear sustained drift. +- Sensitivity preference: react early and tolerate false alarms, or fire only on clear sustained drift. A strong preference either way, or "I expect more than one kind of drift", is the cue to consider an ensemble. - Cadence: how many checks before a first detection is wanted, and how many batches per check. Note that the scalar detectors fire on change in either direction; they do not @@ -56,6 +63,14 @@ plainly (that is the non-wired `EvalDetector`'s job). - distribution/variance/shape change without mean movement: KSWIN - abrupt mean shift, fast and cheap: PageHinkley - gradual, mixed, or general default: ADWIN +- more than one drift shape expected, or a sensitivity preference a single detector cannot express: Ensemble over two or three of the above + +Prefer a single detector when one clearly fits. The ensemble costs an update on +every sub-detector per check and is harder to tune, since each sub-detector is +still driven by its own hyperparameters in the same config block. Reach for it +when the shapes genuinely differ (PageHinkley for abrupt jumps plus KSWIN for +variance changes) or when the user wants a deliberate bias they can state as a +voting rule. State the choice and a one-line reason; name the runner-up if it is close. @@ -66,6 +81,15 @@ Pull defaults and semantics from `docs/drift_detectors.md` and scale to the metr - ADWIN: `adwin_delta` is the main sensitivity knob; keep the two thresholds at defaults unless steering the regime split. - KSWIN: `kswin_alpha` for sensitivity; size the windows to retained samples (`stat_size < window_size`). - PageHinkley: `ph_threshold` scale depends on the metric. For a bounded metric in `[0,1]` the default `50` is very large and rarely fires, so start around `1-10` and tune; larger losses need larger thresholds. `ph_delta` is the slack, `ph_min_instances` is warm-up. +- Ensemble: `ensemble_detectors` lists the sub-detector names. Each is built from this same `[drift_detection]` block, so a detector type can appear at most once and still needs its own hyperparameters set here. An empty list, a nested `"EnsembleDetector"`, or an unknown voting name raises `ValueError` at load. `ensemble_voting` sets the bias: + - `any` (alias `or`) fires when any sub-detector fires. Most sensitive; use when a missed drift costs more than a needless CL dispatch. + - `majority` (default) needs strictly more than half. Balanced, but needs 3+ detectors to differ from `unanimous`. + - `unanimous` (aliases `all`, `and`) needs every detector to fire. Most conservative; suppresses small or noisy changes at the cost of latency. + + Note that `drift_score` is the mean of the sub-detector scores and the regime is + a plurality vote, both independent of the voting rule, so the + `adwin_minor_threshold` / `adwin_moderate_threshold` regime split gets diluted by + sub-detectors reporting a score of 0. Set `detection_interval`, `aggregation`, `metric_index`, and `max_stream_updates` from the cadence answers, and explain any non-default value. @@ -88,6 +112,26 @@ adwin_minor_threshold = 0.3 adwin_moderate_threshold = 0.6 ``` +For an ensemble, list the sub-detectors and keep each one's hyperparameters in the +same block: + +```toml +[drift_detection] +detector_name = "EnsembleDetector" +ensemble_detectors = ["ADWINDetector", "PageHinkleyDetector"] +ensemble_voting = "unanimous" +detection_interval = 10 +aggregation = "mean" +metric_index = 0 +reset_after_learning = false +max_stream_updates = 20 + +# Sub-detectors read their usual hyperparameters from this same block +adwin_delta = 0.002 +ph_threshold = 30 +ph_delta = 0.5 +``` + If a config path was given, patch its `[drift_detection]` section, matching the existing file style. @@ -103,12 +147,18 @@ from apeiron.drift_detection.load_drift_detector import load_drift_detector cfg = build_config(['--config', '']) d = load_drift_detector(cfg) print('OK:', type(d).__name__) +print(getattr(d, 'voting', ''), [type(s).__name__ for s in getattr(d, 'detectors', [])]) print(cfg.drift_detection) " ``` (`PYTHONPATH=src` is required so `import apeiron` resolves; the package lives under `src/apeiron`.) +For an ensemble this is worth more than a syntax check: it is where an empty +`ensemble_detectors`, a nested `EnsembleDetector`, an unknown voting name, or an +unknown sub-detector name surfaces as a `ValueError` instead of at run time. The +second print confirms the resolved voting rule and that every sub-detector built. + For a standalone block, validate against an example TOML with `--set` overrides (a full `Config` still needs `[model]`/`[data]`/`[train]`). Report the recommended detector, the reason, the non-default settings, and that the config loaded. @@ -123,5 +173,13 @@ poetry run python -m src.main --config examples/mnist/mnist.toml \ --set drift_detection.ph_threshold=5 ``` -Keep `docs/drift_detectors.md` as the single source of truth; if the doc and this -skill disagree, fix the doc and follow it. +`--set` values go through `json.loads`, so a list needs JSON syntax and shell +quoting: + +```bash +--set 'drift_detection.ensemble_detectors=["ADWINDetector","KSWINDetector"]' +``` + +Precedence when sources disagree: the code in `src/apeiron/drift_detection/` wins, +then `docs/drift_detectors.md`, then this skill. Fix whichever is stale rather than +working around it; this file has been wrong about detector wiring before. diff --git a/CLAUDE.md b/CLAUDE.md index 17b53c5..623750e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,8 +63,9 @@ The `detector_name` config value must be one of the strings the loader accepts | `PageHinkleyDetector` | Page-Hinkley test (river) | ph_min_instances, ph_delta, ph_threshold, ph_alpha | | `ModelPerformanceDetector` | evidently batch analysis | (uses evidently defaults) | | `EvalDetector` | Direct eval comparison (`ModelEvalDetector`) | metric_index | +| `EnsembleDetector` | Voting over sub-detectors | ensemble_detectors, ensemble_voting | -Note: `EnsembleDetector` is recognized by the loader but raises `NotImplementedError` (sub-detector configuration is not wired up yet) -- do not use it. +`EnsembleDetector` builds each name in `ensemble_detectors` from the same `[drift_detection]` block (so a detector type can appear at most once) and combines their verdicts per `ensemble_voting`: `majority`, `any` (alias `or`), or `unanimous` (aliases `all`, `and`). An unknown voting name or an empty detector list raises `ValueError`. ### Available CL Update Modes | Mode | Strategy | Key Params | diff --git a/docs/configurations.md b/docs/configurations.md index 69379b5..5c7833f 100644 --- a/docs/configurations.md +++ b/docs/configurations.md @@ -183,6 +183,19 @@ ph_alpha = 0.9999 | `ph_threshold` | float | Page-Hinkley trigger threshold. | | `ph_alpha` | float | Page-Hinkley forgetting factor. | +Ensemble: + +```toml +detector_name = "EnsembleDetector" +ensemble_detectors = ["ADWINDetector", "KSWINDetector", "PageHinkleyDetector"] +ensemble_voting = "majority" +``` + +| Option | Type | Description | +|----------|------|-------------| +| `ensemble_detectors` | list[str] | Sub-detectors to combine. Each is built from this same `[drift_detection]` block, so a detector type can appear at most once. | +| `ensemble_voting` | str | How sub-detector verdicts combine: `majority` (more than half fire), `any`/`or` (at least one fires), `unanimous`/`all`/`and` (every one fires). | + Details about the drift detection algorithms available can be found in [docs/drift_detectors.md](drift_detectors.md) diff --git a/docs/drift_detectors.md b/docs/drift_detectors.md index 205e7a3..f16535b 100644 --- a/docs/drift_detectors.md +++ b/docs/drift_detectors.md @@ -43,6 +43,8 @@ Defined in `src/drift_detection/detectors/base.py`: | `ph_delta` | `0.005` | Page-Hinkley change magnitude parameter. | | `ph_threshold` | `50` | Page-Hinkley trigger threshold. | | `ph_alpha` | `0.9999` | Page-Hinkley forgetting factor. | +| `ensemble_detectors` | `()` | Sub-detector names combined by `EnsembleDetector`. | +| `ensemble_voting` | `"majority"` | Voting rule: `majority`, `any`/`or`, `unanimous`/`all`/`and`. | ## Detector Selection (`detector_name`) @@ -53,8 +55,7 @@ Defined in `src/drift_detection/detectors/base.py`: - `PageHinkleyDetector` - `ModelPerformanceDetector` - `EvalDetector` (maps to `ModelEvalDetector`) - -`EnsembleDetector` is present as a class but intentionally not wired in the loader and raises `NotImplementedError`. +- `EnsembleDetector` (combines the detectors listed in `ensemble_detectors` under `ensemble_voting`) ## Detector Classes And Options @@ -169,17 +170,38 @@ Integration note: Brief Explanation: -Wraps several sub-detectors and combines their signals under a `voting` rule (`majority`, `unanimous`, `any`, or weighted) so you can trade sensitivity against false-alarm rate: e.g. `any` reacts to the first detector that fires while `unanimous` requires full agreement. It is conceptually useful for robustness, but note it is not currently loadable from config (see integration note). +Wraps several sub-detectors and combines their signals under a `voting` rule so you can trade sensitivity against false-alarm rate: `any` reacts to the first detector that fires, `unanimous` requires full agreement, `majority` sits in between. -Constructor options: +Config options: -- `detectors: list[BaseDriftDetector]`: sub-detectors whose signals are combined; more detectors = more robust but more compute. -- `voting`: `majority`, `unanimous`, `any`, or weighted fallback: how signals combine. `any` = most sensitive (first firing wins), `majority` = balanced, `unanimous` = most conservative (all must agree). -- `name` +- `ensemble_detectors`: list of sub-detector names to combine, e.g. `["ADWINDetector", "KSWINDetector"]`. Each is built from the same `[drift_detection]` block, so a given detector type can appear at most once. Required; an empty list raises `ValueError`. +- `ensemble_voting`: how the sub-detector verdicts combine (default `majority`): + - `majority` -- fires when strictly more than half the detectors fire. Balanced. + - `any` (alias `or`) -- fires when at least one detector fires. Most sensitive. + - `unanimous` (aliases `all`, `and`) -- fires only when every detector fires. Most conservative. + Names are case-insensitive; an unrecognized value raises `ValueError` at load time. -Integration note: +Behavior: + +- All sub-detectors are updated on every call (no short-circuiting), so `detector.reset()` and internal windows stay in sync. +- The returned `drift_score` is the mean sub-detector score and `confidence` the mean of the non-`None` sub-detector confidences (`None` if no detector reports one), regardless of the voting rule. +- The regime is a plurality vote over the sub-detector regimes, independent of `drift_detected`. +- `metadata` carries `voting`, `n_votes`, `n_detectors`, and the per-detector verdicts. + +Example: -- Class implementation exists, but dynamic config loading for sub-detectors is not implemented. +```toml +[drift_detection] +detector_name = "EnsembleDetector" +ensemble_detectors = ["ADWINDetector", "KSWINDetector", "PageHinkleyDetector"] +ensemble_voting = "majority" +detection_interval = 10 + +# Sub-detectors read their usual hyperparameters from this same block +adwin_delta = 0.002 +kswin_alpha = 0.005 +ph_threshold = 50 +``` ## How Monitor Uses Detectors diff --git a/src/apeiron/config/configuration.py b/src/apeiron/config/configuration.py index c20b32a..6d229d9 100644 --- a/src/apeiron/config/configuration.py +++ b/src/apeiron/config/configuration.py @@ -162,6 +162,20 @@ class DriftDetectionCfg: ph_threshold: float = 50 ph_alpha: float = 0.9999 + # Ensemble hyperparameters (used when detector_name = "EnsembleDetector") + ensemble_detectors: tuple[str, ...] = () + # "majority" | "any" (alias "or") | "unanimous" (aliases "all", "and") + ensemble_voting: str = "majority" + + def __post_init__(self) -> None: + # TOML arrays arrive as lists; keep the frozen config immutable. + # A bare string (e.g. an unquoted --set that failed JSON parsing) is a + # single detector name, not an iterable of characters. + names = self.ensemble_detectors + if isinstance(names, str): + names = (names,) + object.__setattr__(self, "ensemble_detectors", tuple(names)) + @dataclass(frozen=True) class VisualizationCfg: diff --git a/src/apeiron/drift_detection/detectors/model_performance_detector.py b/src/apeiron/drift_detection/detectors/model_performance_detector.py index 72cf1fd..a26eb32 100644 --- a/src/apeiron/drift_detection/detectors/model_performance_detector.py +++ b/src/apeiron/drift_detection/detectors/model_performance_detector.py @@ -309,6 +309,15 @@ class EnsembleDetector(BaseDriftDetector): Combines signals from multiple detectors to make more robust decisions. """ + VOTING_STRATEGIES = { + "majority": "majority", # more than half of the detectors fire + "any": "any", # at least one detector fires + "or": "any", + "unanimous": "unanimous", # every detector fires + "all": "unanimous", + "and": "unanimous", + } + def __init__( self, detectors: List[BaseDriftDetector], @@ -320,12 +329,23 @@ def __init__( Args: detectors: List of individual detectors - voting: Voting strategy ('majority', 'unanimous', 'any', 'weighted') + voting: Voting strategy. One of 'majority', 'any' (alias 'or'), + or 'unanimous' (aliases 'all', 'and'). name: Detector name """ super().__init__(name) + if not detectors: + raise ValueError("EnsembleDetector requires at least one sub-detector") + + key = voting.strip().lower() + if key not in self.VOTING_STRATEGIES: + raise ValueError( + f"Unknown ensemble voting strategy: {voting!r}. " + f"Expected one of {sorted(self.VOTING_STRATEGIES)}." + ) + self.detectors = detectors - self.voting = voting + self.voting = self.VOTING_STRATEGIES[key] self._is_initialized = all(d._is_initialized for d in detectors) def update(self, value: float, **kwargs) -> DriftSignal: @@ -347,18 +367,19 @@ def update(self, value: float, **kwargs) -> DriftSignal: signals.append(signal) detector_names.append(detector.name) + # Average drift scores + avg_drift_score = np.mean([s.drift_score for s in signals]) + # Combine signals based on voting strategy + n_votes = sum(s.drift_detected for s in signals) if self.voting == "majority": - drift_detected = sum(s.drift_detected for s in signals) > len(signals) / 2 + drift_detected = n_votes > len(signals) / 2 elif self.voting == "unanimous": - drift_detected = all(s.drift_detected for s in signals) + drift_detected = n_votes == len(signals) elif self.voting == "any": - drift_detected = any(s.drift_detected for s in signals) - else: # weighted - use average drift score - drift_detected = bool(np.mean([s.drift_score for s in signals]) > 0.5) - - # Average drift scores - avg_drift_score = np.mean([s.drift_score for s in signals]) + drift_detected = n_votes > 0 + else: + raise ValueError(f"unknown voting strategy: {self.voting!r}") # Determine regime by majority vote regime_votes = [s.regime for s in signals] @@ -367,19 +388,21 @@ def update(self, value: float, **kwargs) -> DriftSignal: # Combine metadata metadata = { "n_detectors": len(signals), + "voting": self.voting, + "n_votes": int(n_votes), "individual_signals": [ {"detector": name, "detected": signal.drift_detected} for name, signal in zip(detector_names, signals) ], } + confidences = [s.confidence for s in signals if s.confidence is not None] + return DriftSignal( regime=regime, - drift_detected=drift_detected, + drift_detected=bool(drift_detected), drift_score=float(avg_drift_score), - confidence=float( - np.mean([s.confidence for s in signals if s.confidence is not None]) - ), + confidence=float(np.mean(confidences)) if confidences else None, metadata=metadata, ) diff --git a/src/apeiron/drift_detection/load_drift_detector.py b/src/apeiron/drift_detection/load_drift_detector.py index ee2348e..1273559 100644 --- a/src/apeiron/drift_detection/load_drift_detector.py +++ b/src/apeiron/drift_detection/load_drift_detector.py @@ -2,18 +2,16 @@ from apeiron.drift_detection.detectors.base import BaseDriftDetector -def load_drift_detector(cfg: Config) -> BaseDriftDetector: - """Dynamically load and instantiate a drift detector based on its name. +def _build_detector(detector_name: str, cfg: Config) -> BaseDriftDetector: + """Instantiate a single (non-ensemble) drift detector from the config. Args: - detector_name (str): Name of the drift detector class to load. + detector_name (str): Name of the drift detector class to build. cfg: Configuration object containing parameters for the detector. Returns: BaseDriftDetector: An instance of the specified drift detector. """ - detector_name = cfg.drift_detection.detector_name - detector_instance: BaseDriftDetector if detector_name == "ADWINDetector": from apeiron.drift_detection.detectors.statistical_detectors import ( @@ -52,19 +50,6 @@ def load_drift_detector(cfg: Config) -> BaseDriftDetector: ) detector_instance = ModelPerformanceDetector() - elif detector_name == "EnsembleDetector": - raise NotImplementedError( - "EnsembleDetector requires configuration of sub-detectors, " - "which is not yet implemented. Use ADWINDetector, KSWINDetector, " - "PageHinkleyDetector, or ModelPerformanceDetector instead." - ) - - # from apeiron.drift_detection.detectors.model_performance_detector import ( - # EnsembleDetector, - # ) - - # detector_instance = EnsembleDetector() - elif detector_name == "EvalDetector": from apeiron.drift_detection.detectors.model_performance_detector import ( ModelEvalDetector, @@ -75,3 +60,37 @@ def load_drift_detector(cfg: Config) -> BaseDriftDetector: raise ValueError(f"Unknown drift detector: {detector_name}") return detector_instance + + +def load_drift_detector(cfg: Config) -> BaseDriftDetector: + """Dynamically load and instantiate a drift detector based on its name. + + Args: + cfg: Configuration object containing parameters for the detector. + + Returns: + BaseDriftDetector: An instance of the specified drift detector. + """ + detector_name = cfg.drift_detection.detector_name + + if detector_name != "EnsembleDetector": + return _build_detector(detector_name, cfg) + + from apeiron.drift_detection.detectors.model_performance_detector import ( + EnsembleDetector, + ) + + sub_names = cfg.drift_detection.ensemble_detectors + if not sub_names: + raise ValueError( + "EnsembleDetector requires [drift_detection] ensemble_detectors to list " + "at least one sub-detector, e.g. " + 'ensemble_detectors = ["ADWINDetector", "KSWINDetector"]' + ) + if "EnsembleDetector" in sub_names: + raise ValueError("EnsembleDetector cannot be nested inside itself") + + return EnsembleDetector( + detectors=[_build_detector(name, cfg) for name in sub_names], + voting=cfg.drift_detection.ensemble_voting, + ) diff --git a/tests/test_drift_detection.py b/tests/test_drift_detection.py index 1fec30c..78595eb 100644 --- a/tests/test_drift_detection.py +++ b/tests/test_drift_detection.py @@ -6,6 +6,7 @@ import pytest from apeiron.drift_detection.detectors.base import ( + BaseDriftDetector, DriftSignal, LearningRegime, ) @@ -263,10 +264,33 @@ def test_drift_when_metric_drops(self): # --------------------------------------------------------------------------- # EnsembleDetector # --------------------------------------------------------------------------- +class FakeDetector(BaseDriftDetector): + """Detector that always reports a fixed verdict, for voting tests.""" + + def __init__(self, detected: bool, name: str = "Fake"): + super().__init__(name) + self.detected = detected + + def update(self, value: float, **kwargs) -> DriftSignal: + return DriftSignal( + regime=LearningRegime.CONTINUAL_LEARNING + if self.detected + else LearningRegime.STABLE, + drift_detected=self.detected, + drift_score=1.0 if self.detected else 0.0, + ) + + def reset(self) -> None: + pass + + class TestEnsembleDetector: def _make_detectors(self, n=3): return [ADWINDetector(delta=0.002) for _ in range(n)] + def _votes(self, *detected: bool): + return [FakeDetector(d) for d in detected] + def test_majority_voting(self): detectors = self._make_detectors(3) ensemble = EnsembleDetector(detectors, voting="majority") @@ -286,6 +310,40 @@ def test_unanimous_voting(self): signal = ensemble.update(1.0) assert isinstance(signal, DriftSignal) + @pytest.mark.parametrize( + "voting,votes,expected", + [ + ("majority", (True, True, False), True), + ("majority", (True, False, False), False), + ("any", (True, False, False), True), + ("any", (False, False, False), False), + ("unanimous", (True, True, True), True), + ("unanimous", (True, True, False), False), + ], + ) + def test_voting_strategies(self, voting, votes, expected): + ensemble = EnsembleDetector(self._votes(*votes), voting=voting) + signal = ensemble.update(1.0) + assert signal.drift_detected is expected + assert signal.metadata["voting"] == voting + assert signal.metadata["n_votes"] == sum(votes) + + @pytest.mark.parametrize( + "alias,canonical", + [("all", "unanimous"), ("and", "unanimous"), ("or", "any")], + ) + def test_voting_aliases(self, alias, canonical): + ensemble = EnsembleDetector(self._votes(True, False), voting=alias) + assert ensemble.voting == canonical + + def test_unknown_voting_raises(self): + with pytest.raises(ValueError, match="voting"): + EnsembleDetector(self._votes(True), voting="plurality") + + def test_empty_detectors_raises(self): + with pytest.raises(ValueError, match="at least one"): + EnsembleDetector([], voting="any") + def test_reset_resets_all(self): detectors = self._make_detectors(2) ensemble = EnsembleDetector(detectors, voting="majority") @@ -350,14 +408,30 @@ def test_eval_detector(self, default_cfg): d = load_drift_detector(cfg) assert isinstance(d, ModelEvalDetector) - def test_ensemble_not_implemented(self, default_cfg): + def test_ensemble(self, default_cfg): + from dataclasses import replace + + cfg = replace( + default_cfg, + drift_detection=DriftDetectionCfg( + detector_name="EnsembleDetector", + ensemble_detectors=["ADWINDetector", "KSWINDetector"], + ensemble_voting="any", + ), + ) + d = load_drift_detector(cfg) + assert isinstance(d, EnsembleDetector) + assert d.voting == "any" + assert [type(sub) for sub in d.detectors] == [ADWINDetector, KSWINDetector] + + def test_ensemble_without_sub_detectors_raises(self, default_cfg): from dataclasses import replace cfg = replace( default_cfg, drift_detection=DriftDetectionCfg(detector_name="EnsembleDetector"), ) - with pytest.raises(NotImplementedError): + with pytest.raises(ValueError, match="ensemble_detectors"): load_drift_detector(cfg) def test_unknown_detector_raises(self, default_cfg):