Skip to content

Add a new project focused on Darts library + timeseries forecasting #241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 26 commits into from
Aug 20, 2025

Conversation

htahir1
Copy link
Contributor

@htahir1 htahir1 commented Aug 19, 2025

Summary

Please provide a short summary explaining the motivation behind these changes.

Checklist

  • I have read the contributing guidelines.
  • I have run the necessary tests and linters.
  • I have updated relevant documentation where applicable.

Related Issues

Please link to any relevant issues or discussions.

@htahir1 htahir1 requested a review from strickvl August 19, 2025 12:30
Copy link

dagshub bot commented Aug 19, 2025

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

@strickvl strickvl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so some bigger things and then some comments also attached with nits.

Inference Pipeline Doesn't Use the Trained Scaler

The training data is scaled before the model sees it, but the inference pipeline feeds un-scaled data to the model. This will cause the model to produce incorrect and nonsensical predictions.

The fix:

  • Modify the training preprocessing step to link the fitted Scaler object to the ZenML model version
  • Remove the incorrect preprocessing step from the inference pipeline.
  • Update the batch inference step to load the Scaler from the model version and apply it to the new data before prediction.

Duplicated code

  • iterative prediction logic is duplicated in steps/evaluate.py and steps/batch_infer.py
  • (This duplicated iterative prediction logic also contains a bug where the prediction chunk size is hardcoded to 14, which might not match the model's output_chunk_length)

@htahir1
Copy link
Contributor Author

htahir1 commented Aug 19, 2025

@strickvl good catch on the scaler. i fixed this

@htahir1 htahir1 requested a review from strickvl August 19, 2025 20:00
@strickvl
Copy link
Contributor

@htahir1 needs formatting to pass the tests, though...

@htahir1 htahir1 merged commit 43aefb4 into main Aug 20, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants