Conversation
Codecov Report
@@ Coverage Diff @@
## main #3794 +/- ##
=====================================
Coverage 99.7% 99.7%
=====================================
Files 343 343
Lines 35797 35797
=====================================
Hits 35660 35660
Misses 137 137
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
eccabay
requested review from
christopherbunn,
chukarsten and
jeremyliweishih
| from evalml.utils.gen_utils import contains_all_ts_parameters | ||
|
|
||
| _UNSUPPORTED_FREQUENCIES_STL_DECOMPOSER = ["A", "T"] | ||
| _UNSUPPORTED_FREQUENCIES_STL_DECOMPOSER = ["A", "Y", "T", "AS", "YS"] |
There was a problem hiding this comment.
For reference: A and Y are both year end, AS and YS are year start, T is minute
chukarsten
left a comment
chukarsten
left a comment
There was a problem hiding this comment.
No blocking comments, but looks good. I do think each decomposer module should be cognizant of what frequencies are no bueno for it. And I'll leave it up to you whether you think the child-decomposer class has a method to return that.
| "frequency, should_decomp", | ||
| [ | ||
| ("D", True), | ||
| ("MS", True), | ||
| ("A", False), | ||
| ("T", False), | ||
| ("10T", False), | ||
| ("AS-JAN", False), | ||
| ("YS", False), | ||
| ], | ||
| ) |
There was a problem hiding this comment.
I suspect this is going to be on a per-decomposer basis...so we might need to do this differently going forward, when we introduce new decomposers and also introduce new ways of trying more pipelines with different decomposers. I think it's fine for now and we can kick the can to when we do X11 or X13.
Could we also add mention that this is a test for time series only in the test name?
| from evalml.utils.gen_utils import contains_all_ts_parameters | ||
|
|
||
| _UNSUPPORTED_FREQUENCIES_STL_DECOMPOSER = ["A", "T"] | ||
| _UNSUPPORTED_FREQUENCIES_STL_DECOMPOSER = ["A", "Y", "T", "AS", "YS"] |
There was a problem hiding this comment.
we might want to move this either 1.) into the stl_decomposer module and/or 2.) make it returnable by the decomposer itself. Then we can dynamically know what decomposers support or do not. You could also build the parametrization list and parametrize across STL decomposers in your decomposer test that way.
Part of the motivation behind the move, in my mind, is the answer to "where should I go to find what frequencies are not supported by X Decomposer?"
There was a problem hiding this comment.
A lovely idea - filed #3795 to track this!
3 participants
Fixes bug where any invalid frequencies that were either offset (i.e. 10 minutes ->
10T) or anchored (annually at the year end ->A-DEC) were not properly getting excluded from training with the decomposer.