[FIX] UCM: remove dead _calculate_sigma helper and unused sigma storage#1136
Closed
shaun0927 wants to merge 1 commit into
Closed
[FIX] UCM: remove dead _calculate_sigma helper and unused sigma storage#1136shaun0927 wants to merge 1 commit into
shaun0927 wants to merge 1 commit into
Conversation
UCM stored model_["sigma"] but no method (predict, predict_in_sample, forecast) read it; intervals come from statsmodels `forecast.conf_int` directly, and UCM does not implement `simulate()`. The local _calculate_sigma also shadowed statsforecast.utils._calculate_sigma with a different denominator (max(n-1, 1) instead of the convention n - n_params used by the 30+ call sites in models.py). A follow-up can wire UCM into the simulate API from Nixtla#1072 if/when that's planned.
Contributor
|
We are refactoring the UCM model in #1158 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UCMdefines a private_calculate_sigmahelper atucm.py:25and stores its result inmodel_["sigma"]atucm.py:292, but no method ever reads it.predict,predict_in_sample, andforecastall retrieve intervals fromforecast.conf_int(alpha=…)(statsmodels) directly. UCM does not implementsimulate()either, so the simulation pattern from #1072 (which consumesmodel_["sigma"]) is not in play.The helper is therefore dead code with two further problems:
statsforecast.utils._calculate_sigma, which is used by 30+ call sites inmodels.py.max(n - 1, 1)instead ofn) — and the convention everywhere else is_calculate_sigma(residuals, len(residuals) - n_params), i.e. the caller pre-subtracts the parameter count.Reproducer
Changes
This PR removes the dead path. A follow-up can wire UCM into
simulate()later (following the pattern from #1072), at which point the canonicalfrom statsforecast.utils import _calculate_sigmashould be imported and the correctn - n_paramsdenominator used.