zalandoresearch / pytorch-ts

PyTorch based Probabilistic Time Series forecasting framework based on GluonTS backend
MIT License
1.21k stars 190 forks source link

Potential bug in calculating `lags_for_fourier_time_features_from_frequency` function #42

Closed hashbangCoder closed 3 years ago

hashbangCoder commented 3 years ago

Hello,

I was trying to understand the code execution and stumbled across a potential bug. The lags_for_fourier_time_features_from_frequency() returns an incorrect result when you pass a minute level freq arg.

Example : lags_for_fourier_time_features_from_frequency(freq='10min') == [1] when it should return [1, 4, 12, 24, 48]

For other frequencies it works as expected -

Could you please explain why max(lags) is added to self.history_length to increase context len in here ?

kashif commented 3 years ago

@hashbangCoder so yes the issue with the lags_for_fourier_time_features_from_frequency for min freq. should be fixed now...

regarding why max(lags) is added to the context is because we need to paste in the values from some time back and so want to make sure that the window is big enough given the lag features indicies... hope that answers it?