Conversation
…egy pattern. added architecture documentation Signed-off-by: Christoph Huy <christoph.huy@campus.tu-berlin.de>
Signed-off-by: Christoph Huy <christoph.huy@campus.tu-berlin.de>
There was a problem hiding this comment.
Pull request overview
This PR refactors the MAD (Median Absolute Deviation) anomaly detection implementation to improve modularity and extensibility. The changes introduce a strategy pattern for scoring algorithms and consolidate decomposition-based detection into a unified class.
- Introduced
MadScorerinterface withGlobalMadScorerandRollingMadScorerimplementations - Consolidated STL-based detection into
DecompositionMadAnomalyDetectionsupporting both STL and MSTL - Reorganized code structure by moving MAD components into a dedicated subdirectory
Reviewed changes
Copilot reviewed 5 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/sdk/python/rtdip_sdk/pipelines/anomaly_detection/spark/mad_anomaly_detection.py |
Removed original file containing three separate detector classes |
src/sdk/python/rtdip_sdk/pipelines/anomaly_detection/spark/mad/mad_anomaly_detection.py |
New implementation with refactored MadAnomalyDetection and DecompositionMadAnomalyDetection classes using scorer strategy pattern |
src/sdk/python/rtdip_sdk/pipelines/anomaly_detection/spark/mad/interfaces.py |
New abstract MadScorer interface defining the strategy pattern for scoring algorithms |
tests/sdk/python/rtdip_sdk/pipelines/anomaly_detection/spark/test_mad.py |
Updated tests to use new class names and scorer implementations with added parametrized tests for decomposition methods |
amos_team_resources/anomaly_detection/uml/mad_arch.xml |
Added UML diagram documenting the new architecture |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from .interfaces import MadScorer | ||
|
|
||
|
|
||
| class GlobalMadScorer(MadScorer): |
There was a problem hiding this comment.
Class name inconsistency: The class is named GlobalMadScorer but the UML diagram references it as MadGlobalScorer. Consider renaming to MadGlobalScorer to match the documentation.
| return 0.6745 * (series - median) / mad | ||
|
|
||
|
|
||
| class RollingMadScorer(MadScorer): |
There was a problem hiding this comment.
Class name inconsistency: The class is named RollingMadScorer but the UML diagram references it as MadRollingScorer. Consider renaming to MadRollingScorer to match the documentation.
|
|
||
| def test_mad_anomaly_detection(spark_dataframe_with_anomalies): | ||
| def test_mad_anomaly_detection_global(spark_dataframe_with_anomalies): | ||
| scorer = GlobalMadScorer() |
There was a problem hiding this comment.
The scorer variable is instantiated but never used. The MadAnomalyDetection class already creates a default GlobalMadScorer when no scorer is provided. Either pass this scorer to the detector or remove this line.
pr for sprint09