Fix off-by-one in torch DeepAR

[lostella]

Issue #, if available: Fixes #2616

Description of changes: The RNN in torch DeepAR was getting dynamic features shifted by one as input.

• Split prepare_rnn_input out of unroll_lagged_rnn
• Add test to verify alignment of RNN inputs (target values vs dynamic features, test is failing prior to fix)
• Change is breaking in that previously serialized models will not work as before (network changes)

Todo:

• Take the opportunity to improve the code
• Test effect of bug/fix (e.g. simple indicator that zeroes-out a series, model should not get it right prior to fix)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Please tag this pr with at least one of these labels to make our release process faster: BREAKING, new feature, bug fix, other change, dev setup

[kashif]

is there a need to append this when there is no future_target

[lostella]

Yes, future_time_feat is not an Optional argument anymore (it never should have been, probably). The assumption is that at least one "future output" will be produced, for which the corresponding dynamic features are needed. In fact, it is assumed that there's one less future_target than future_time_feat, if any, see lines 211-215 above. Let me improve this a bit

[kashif]

BTW @lostella do you think the same issue also affects the transformer model? https://github.com/awslabs/gluonts/blob/dev/src/gluonts/mx/model/transformer/_network.py#L172-L178

[lostella]

BTW @lostella do you think the same issue also affects the transformer model? https://github.com/awslabs/gluonts/blob/dev/src/gluonts/mx/model/transformer/_network.py#L172-L178

It's hard to say. The easiest way to verify this, I think, is to set up a small unit test what the input to whatever layer will be: in this case the tensor is split into encoder + decoder input, so one could have a method prepare those two directly and then test that lagged target values, dynamic features, and what not, are aligned as expected.

[jaheba]

I think I prefer reshape(1, history_length, 1) instead of unsqueeze. Makes it more clear what the output shapes is in my view.

[kashif]

or even .view()

[lostella]

For some reason this test started failing again with

E       RuntimeError: element 0 of tensors does not require grad and does not have a grad_fn

../.pyenv/versions/3.8.13/envs/gluonts/lib/python3.8/site-packages/torch/autograd/__init__.py:197: RuntimeError

I think we need to understand this better and fix it separately.

[lostella]

This kind of error kept #2496 on hold for a while, then it disappeared on its own. But something must be off.

[mbohlkeschneider]

Maybe I'm completely not getting the issue here, but if we do l-1 for the lags, don't we feed in the current time stamp during training when l=1 thereby introducing a data leak? Maybe I'm wrong, though.

[lostella]

It depends on what time index the lag is applied: for how the RNN input preparation is written, the lag index gets subtracted from t, when we have data up to time t and want a prediction for time t+1. The test_rnn_input test function aims at verifying precisely this. Let me try to see whether the test can be made more apparent.

[kashif]

@lostella one issue i found with the lagged_sequence_values helper is that with multivariate inputs this helper doesn't return the stacked tensors... e.g.

# prior_input.shape torch.Size([1, 167, 137])
# input.shape torch.Size([1, 24, 137])

lags = lagged_sequence_values([0, 167], prior_input, input, dim=1)
# lags.shape torch.Size([1, 24, 137, 2])

where i would have expected torch.Size([1, 24, 137*2])