[BUG] pytorch-forecasting#1752 Fixing

[RUPESH-KUMAR01]

Description

This PR is towards the issue #1752 . concat_sequences concat the timesteps of each batch. But our goal is to not concat time_stamps but to concat the batches.

Checklist

• Linked issues (if existing)
• Amended changelog for large changes (and added myself there as contributor)
• Added/modified tests
• Used pre-commit hooks when committing to ensure that code is compliant with hooks. Install hooks with pre-commit install.
  To run hooks independent of commit, execute pre-commit run --all-files

[fkiraly]

Great - could we add a test to ensure we have fixed the bug?

[RUPESH-KUMAR01]

Changes Made to include the test for the bug :

1. Changed fast_dev_run to 2

   • Previously, only one batch was run, which might have hidden errors.
   • Now, two batches will run to ensure better testing.

2. Added an assertion to check y shape after concatenation

   • Ensures that predictions match target dimensions after processing.

Updated Code Snippet:

predictions = net.predict(
    val_dataloader,
    return_index=True,
    return_x=True,
    return_y=True,
    fast_dev_run=2,  # 🔹 Now runs for two batches
    trainer_kwargs=trainer_kwargs,
)

if isinstance(predictions.output, torch.Tensor):
    assert predictions.output.shape == predictions.y[0].shape, "shape of predictions should match shape of targets"
else:
    for i in range(len(predictions.output)):
        assert predictions.output[i].shape == predictions.y[0][i].shape, "shape of predictions should match shape of targets"

I am not familiar with tests, but while debugging, I found where the tests failed with my previous approach and modified the function with extra constraints.
Location of the file where I modified:
tests\test_models\test_temporal_fusion_transformer.py
function: _integration

If the changes are sufficient I will add the modifications to the PR.

[fkiraly]

Nice, thanks for fixing!

Some questions:

It looks like you are changing the tests, not just the code in _utils.

Can you explain that?

Would the original test fail?

If not, we should not change existing tests, but add new ones for the bug.

[Hspix]

The fix is pending？

[RUPESH-KUMAR01]

I tested the code initially before making changes in tests, which did not fail. The tests I modified are to add more constraints to the initial tests. The test modifications will ensure that the batch_size and time_stamps of both output and y match.