Skip to content

Fix SR-CNN missing anomalies when Period > 0#7610

Open
FanBaoMS wants to merge 3 commits intodotnet:mainfrom
FanBaoMS:fix/srcnn-anomaly-detection
Open

Fix SR-CNN missing anomalies when Period > 0#7610
FanBaoMS wants to merge 3 commits intodotnet:mainfrom
FanBaoMS:fix/srcnn-anomaly-detection

Conversation

@FanBaoMS
Copy link
Copy Markdown

@FanBaoMS FanBaoMS commented May 5, 2026

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

Fixes #5891 - DetectEntireAnomalyBySrCnn - No anomalies detected as of version 1.5.4 or greater

Root cause

Two domain-mismatch bugs in the Period > 0 code path of SrCnnEntireModeler:

1. Train() — z-score statistics computed in the wrong domain.
_mean / _std were computed from the raw input series before deseasonality ran. The z-score "false anomaly" filter inside SpectralResidual then compared _seriesToDetect[i] (the deseasonalized residual) against those raw-domain stats, producing meaningless z-scores that suppressed real anomalies.

2. GetMarginPeriod()CalculateAnomalyScore called with exp in the wrong domain.
_ifftRe[i] was passed as the expected-value argument. CalculateAnomalyScore computes |exp - value| / unit, which is only meaningful when both arguments share a domain.

In the no-period sibling GetMargin(), this call is correct: CalculateExpectedValueByFft(_deAnomalyData) is invoked immediately beforehand and overwrites _ifftRe with the raw-domain expected value of the de-anomalized series — so _ifftRe[i] and values[i] share a domain there.

GetMarginPeriod() was added later (#5202) and reused the same call shape without that prior overwrite. In the period > 0 path _ifftRe holds the SR saliency map's real component computed on the deseasonalized residual series, not a raw-domain expected value. The resulting score collapses to roughly |raw_value| / unit — proportional to raw magnitude rather than deviation from expectation — so real anomalies often score near zero. This looks like a copy-paste of the no-period call site without re-deriving what _ifftRe actually holds in the new path.

Fix

  1. Move the sum / _mean / _std computation in Train() to after the deseasonality call and compute the statistics against _seriesToDetect (the same series SR will actually score).
  2. In GetMarginPeriod() pass results[i][3] (the raw-domain expected value just produced by GetExpectedValuePeriod) instead of _ifftRe[i].

The no-period path (Train with _period == 0 and GetMargin) is correct as written and is unchanged.

Test

Adds TestSrCnnAnomalyDetectorPhoneCalls, ported from the dotnet/samples PhoneCallsAnomalyDetection tutorial. Data file is taken verbatim from dotnet/samples.

The test asserts DetectSeasonality returns 7 and that DetectEntireAnomalyBySrCnn flags exactly indices {28, 44, 56, 70}. Prior to this fix the same call detects zero anomalies on the same input.

Sensitivity = 87.0 is used to obtain the boundary width the tutorial was originally calibrated for under the v1.5.2 _factors table; the table was rewritten in #5436, so the tutorial's literal Sensitivity = 64.0 now produces a much wider boundary that masks all but the strongest spike at index 28.

FanBaoMS added 2 commits May 5, 2026 14:02
In the period > 0 path, two domain-mismatch bugs caused real anomalies
to be suppressed:

1. SrCnnEntireModeler.Train computed _mean / _std from the raw input
   series before deseasonality ran. SpectralResidual then compared the
   deseasonalized residuals in _seriesToDetect against those raw-domain
   statistics, producing meaningless z-scores in the false-anomaly
   filter. Move the sum/mean/std computation below the deseasonality
   call and compute the statistics against _seriesToDetect.

2. SrCnnEntireModeler.GetMarginPeriod passed _ifftRe[i] to
   CalculateAnomalyScore as the expected value. _ifftRe in this path
   is the SR saliency map's real component computed from the
   deseasonalized residual series, not a raw-domain expected value.
   CalculateAnomalyScore expects exp and value in the same domain, so
   the resulting score was proportional to raw magnitude rather than
   deviation from expectation. Pass results[i][3] (the raw-domain
   expected value produced by GetExpectedValuePeriod above) instead.

The no-period path (Train with _period == 0 and GetMargin) is correct
as written and is unchanged.
Adds TestSrCnnAnomalyDetectorPhoneCalls, a regression test based on
the dotnet/samples PhoneCallsAnomalyDetection tutorial. The test data
file is taken verbatim from the dotnet/samples repository.

DetectSeasonality is asserted to return period 7 (the tutorial's
documented output). DetectEntireAnomalyBySrCnn is then expected to
flag indices {28, 44, 56, 70} and only those. Prior to the
SrCnnEntireModeler period-path fix this test detects no anomalies
on the same input.

Sensitivity = 87.0 is used to obtain the boundary width the tutorial
was originally calibrated for under the v1.5.2 _factors table; the
table was rewritten in a later release so the tutorial's literal
Sensitivity = 64.0 now produces a much wider boundary.
@FanBaoMS
Copy link
Copy Markdown
Author

FanBaoMS commented May 5, 2026

@dotnet-policy-service agree company="Microsoft"

@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.60%. Comparing base (4c8b357) to head (197a859).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7610   +/-   ##
=======================================
  Coverage   69.59%   69.60%           
=======================================
  Files        1484     1484           
  Lines      273606   273641   +35     
  Branches    27949    27952    +3     
=======================================
+ Hits       190408   190457   +49     
+ Misses      75836    75822   -14     
  Partials     7362     7362           
Flag Coverage Δ
Debug 69.60% <100.00%> (+<0.01%) ⬆️
production 63.85% <100.00%> (+<0.01%) ⬆️
test 89.64% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rosoft.ML.TimeSeries/SrCnnEntireAnomalyDetector.cs 93.51% <100.00%> (+1.14%) ⬆️
...crosoft.ML.TimeSeries.Tests/TimeSeriesDirectApi.cs 99.39% <100.00%> (+0.02%) ⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DetectEntireAnomalyBySrCnn - No anomalies detected as of version 1.5.4 or greater

1 participant