Skip to content

Update the EnsembleDetector with multiple voting system - #114

Merged
anagainaru merged 5 commits into
mainfrom
multiple_detectors
Aug 5, 2026
Merged

Update the EnsembleDetector with multiple voting system#114
anagainaru merged 5 commits into
mainfrom
multiple_detectors

Conversation

@anagainaru

@anagainaru anagainaru commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

The EnsembleDetector existed before but was not usable, it also only accepted a voting majority as the strategy to decide when to fire. This PR extends it so it can accept two more strategies (any reacts to the first detector that fires, unanimous requires full agreement, majority sits in between).

This PR also fixes the lack of configuration options that made the EnsembleDetector not usable. After the current PR toml files can include the following options:

detector_name = "EnsembleDetector"
ensemble_detectors = ["ADWINDetector", "KSWINDetector", "PageHinkleyDetector"]
ensemble_voting = "majority"

EnsembleDetector builds each name in ensemble_detectors from the same [drift_detection] block (so a detector type can appear at most once). An unknown voting name or an empty detector list raises ValueError.

SLAC model harness (from PR #83) with EWC and KSWin fires 4 times:
image

with PageHinkley fires 2 times:
image

And with both (and any strategy) fires 6 times:
image

@anagainaru
anagainaru requested a review from rz4 August 4, 2026 14:26
@anagainaru
anagainaru marked this pull request as ready for review August 4, 2026 14:27
@anagainaru
anagainaru requested a review from andrewfayres August 5, 2026 13:08

@andrewfayres andrewfayres left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small nitpick


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:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Config still should still hold the detector name in it. We should continue to derive detector name from the config instead of accepting it separately. It's slightly less error prone and a bit easier to use.

Changing it to this means every caller will do:
detector_name = cfg.drift_detection.detector_name
_build_detector(detector_name, cfg)

instead of:
_build_detector(cfg)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The load_drift_detection function is still using the detector name set in config. This change is necessary (i.e. splitting the load function into _build_detector that takes a given detector) to cover the EnsembleDetector. For the EnsembleDetector the config is like this:

detector_name = "EnsembleDetector"
ensemble_detectors = ["ADWINDetector", "KSWINDetector", "PageHinkleyDetector"]
ensemble_voting = "majority"

which means we are calling _build_detector for each detector in the list.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay so for the ensemble detector, the name that we're passing to buld_detector will differ from the name in the config? That's what it looks like based on the below code snippet. Odd behavior but it's an internal only method so I suppose it's fine.

detectors=[_build_detector(name, cfg) for name in sub_names]

andrewfayres
andrewfayres previously approved these changes Aug 5, 2026

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:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay so for the ensemble detector, the name that we're passing to buld_detector will differ from the name in the config? That's what it looks like based on the below code snippet. Odd behavior but it's an internal only method so I suppose it's fine.

detectors=[_build_detector(name, cfg) for name in sub_names]

Comment thread src/apeiron/drift_detection/detectors/model_performance_detector.py Outdated
@anagainaru

Copy link
Copy Markdown
Collaborator Author

@andrewfayres we basically check if the name is EnsembleDetector and if yes we go over the list of detectors in the ensemble and build one for each entry. For any other name we just build the given detector.

    detector_name = cfg.drift_detection.detector_name
    if detector_name != "EnsembleDetector":
        return _build_detector(detector_name, cfg)

The other option was to allow the name of the detector to be a list, in which case we would still need to handle it differently depending on if a string or a list was given. Keeping a separate EnsembleDetector allows me to have configuration options for it (e.g. the voting method).

Co-authored-by: Rafael Zamora-Resendiz <15003285+rz4@users.noreply.github.com>
@anagainaru
anagainaru merged commit dc9fd7f into main Aug 5, 2026
3 checks passed
@anagainaru
anagainaru deleted the multiple_detectors branch August 5, 2026 20:37
S-Villar pushed a commit that referenced this pull request Aug 6, 2026
Adds the singles-vs-ensemble result: over the identical 343-window trace, ADWIN
and Page-Hinkley never fire, KSWIN fires four times, and an any-vote ensemble of
all three fires at exactly KSWIN's steps. The shift is in the distribution of
the per-window error rather than its mean, which is what the two mean-based
detectors miss. The ensemble is reported as the negative result it is: no false
alarms added, no sensitivity gained.

The figure follows #114's layout -- regimes shaded and named, and a headline
block giving the pretrained base model against the adapted one. The percentage
now reads "lower" rather than carrying a sign that looked like error rising, and
the per-adaptation drops say they are measured on the arriving bundle's
validation split, not on the streamed windows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
S-Villar pushed a commit that referenced this pull request Aug 6, 2026
Adds the singles-vs-ensemble result: over the identical 343-window trace, ADWIN
and Page-Hinkley never fire, KSWIN fires four times, and an any-vote ensemble of
all three fires at exactly KSWIN's steps. The shift is in the distribution of
the per-window error rather than its mean, which is what the two mean-based
detectors miss. The ensemble is reported as the negative result it is: no false
alarms added, no sensitivity gained.

The figure follows #114's layout -- regimes shaded and named, and a headline
block giving the pretrained base model against the adapted one. The percentage
now reads "lower" rather than carrying a sign that looked like error rising, and
the per-adaptation drops say they are measured on the arriving bundle's
validation split, not on the streamed windows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
S-Villar pushed a commit that referenced this pull request Aug 6, 2026
Adds the singles-vs-ensemble result: over the identical 343-window trace, ADWIN
and Page-Hinkley never fire, KSWIN fires four times, and an any-vote ensemble of
all three fires at exactly KSWIN's steps. The shift is in the distribution of
the per-window error rather than its mean, which is what the two mean-based
detectors miss. The ensemble is reported as the negative result it is: no false
alarms added, no sensitivity gained.

The figure follows #114's layout -- regimes shaded and named, and a headline
block giving the pretrained base model against the adapted one. The percentage
now reads "lower" rather than carrying a sign that looked like error rising, and
the per-adaptation drops say they are measured on the arriving bundle's
validation split, not on the streamed windows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Zilinghan Zilinghan mentioned this pull request Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants