pytorch-forecasting icon indicating copy to clipboard operation
pytorch-forecasting copied to clipboard

[BUG] pytorch-forecasting#1752 Fixing

Open RUPESH-KUMAR01 opened this issue 9 months ago • 3 comments

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

  • [x] 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

RUPESH-KUMAR01 avatar Feb 26 '25 15:02 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.

RUPESH-KUMAR01 avatar Feb 27 '25 09:02 RUPESH-KUMAR01

The fix is pending?

Hspix avatar Mar 12 '25 08:03 Hspix

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.

RUPESH-KUMAR01 avatar Mar 12 '25 10:03 RUPESH-KUMAR01