Fix wrong seasonality basis generator for NBEATS #804
Conversation
vladimir-chernykh
requested review from
hrzn,
tomasvanpottelbergh,
pennfranc,
dennisbader and
brunnedu
as code owners
There was a problem hiding this comment.
Hi @vladimir-chernykh and thanks a lot for pointing this out!
I checked and agree with you. That one was easy to overlook!
I initiated the tests
Codecov Report
@@ Coverage Diff @@
## master #804 +/- ##
=======================================
Coverage 91.04% 91.04%
=======================================
Files 69 69
Lines 6858 6858
=======================================
Hits 6244 6244
Misses 614 614
Continue to review full report at Codecov.
|
|
I believe the range of
What you're talking about I think is the theta coefficients. For cosines we take first floor(H/2-1) values from theta vector and for sines we take next floor(H/2-1) values starting from floor(H/2). But this is already done correctly in the DARTS. So in my opinion no more changes needed here. |
You are right - I convinced myself too between my earlier comment and now. Thanks for checking - and good catch! We will release it (along with a planned patch release) asap. |
Fixes #803.
Summary
It fixes wrong seasonality basis vectors generated for NBEATS interpretable model. Now the basis vectors are normalized by the horizon length as it is done in the paper.
Other Information
--