Skip to content

Fix Bartlett significance threshold in seasonality detection - #3288

Open
its-amann wants to merge 3 commits into
unit8co:masterfrom
its-amann:fix/check-seasonality-bartlett-3286
Open

its-amann wants to merge 3 commits into
unit8co:masterfrom
its-amann:fix/check-seasonality-bartlett-3286

Conversation

@its-amann

Copy link
Copy Markdown

Checklist before merging this PR:

  • Linked the issue addressed by this PR.
  • Summarized the changes below.
  • Added an Unreleased changelog entry.

Fixes #3286.

Summary

check_seasonality() scales Bartlett's standard error by the empirical mean and variance of the ACF. For white noise, that multiplier can be close to zero or negative, making insignificant peaks look seasonal.

This uses the normal critical value directly and passes the actual lag to _bartlett_formula(), so its sum includes all preceding nonzero-lag autocorrelations. Public signatures, candidate selection and return conventions are unchanged.

Regression tests cover seeded white noise, Bartlett confidence intervals from statsmodels at three significance levels, noisy periods 2/6/12, and default Theta/FourTheta fitting and forecasting.

The corrected test also changes automatic detection on the trending AirPassengers benchmark: its lag-12 ACF is below the corrected cutoff. Those forecasting fixtures now specify the known annual period explicitly, keeping their existing accuracy limits. Default automatic detection is tested separately. The changelog notes that inferred periods and forecasts can change; there is no new detrending or multiple-testing correction.

Thanks @akshpatel0910 for the detailed reproduction.

Other Information

  • Linux, Python 3.12.13 / pandas 3.0.6: 3,833 passed, 217 skipped, exit 0, across darts/tests/utils, test_4theta.py and test_local_forecasting_models.py. Skips are optional-dependency cases; no test-selection exclusions were used.
  • Windows: the statistics/Theta modules passed all 68 tests; all 16 model-performance cases and 7 local-model ensemble checks also passed.
  • Configured pre-commit checks passed on all changed files; fresh wheel and source builds passed.
  • Changed-module type diagnostics match the base branch (44 existing diagnostics, no differences).

These are CPU/core-dependency test runs, not the full optional-model CI matrix.

@its-amann

Copy link
Copy Markdown
Author

@dennisbader, the workflow is waiting for maintainer approval. Could you approve the run when you have a chance?

I'd also appreciate your view on the AirPassengers benchmark adjustment discussed in #3286. The PR keeps the formula correction separate from any new detrending logic, and the existing benchmark accuracy limits are unchanged. Happy to adjust the scope based on your feedback.

@codecov

codecov Bot commented Oct 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.13%. Comparing base (9e7f9a0) to head (f256a4a).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3288   +/-   ##
=======================================
  Coverage   97.13%   97.13%           
=======================================
  Files         175      175           
  Lines       19410    19410           
=======================================
  Hits        18853    18853           
  Misses        557      557           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

This branch has not been deployed

No deployments
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.

[BUG] check_seasonality reports seasonality for almost any series, including white noise

1 participant