Skip to content

Conversation

phoeenniixx
Copy link
Member

Description

This PR implements the tft model based on the new data pipeline. This PR is branched off from #1811

@phoeenniixx phoeenniixx changed the title Refactor model [ENH] EXPERIMENTAL: TFT model based on the new data pipeline Apr 6, 2025
Copy link

codecov bot commented Apr 7, 2025

Codecov Report

❌ Patch coverage is 0% with 173 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@4c30351). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...ting/models/temporal_fusion_transformer/_tft_v2.py 0.00% 97 Missing ⚠️
pytorch_forecasting/models/base/_base_model_v2.py 0.00% 76 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1812   +/-   ##
=======================================
  Coverage        ?   84.20%           
=======================================
  Files           ?       53           
  Lines           ?     5836           
  Branches        ?        0           
=======================================
  Hits            ?     4914           
  Misses          ?      922           
  Partials        ?        0           
Flag Coverage Δ
cpu 84.20% <0.00%> (?)
pytest 84.20% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fkiraly
Copy link
Collaborator

fkiraly commented May 13, 2025

Yes maybe this will help some confused souls like mine :)

But you did everything right

Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Great!

May I just ask for minor changes:

  • in the base model, include the warning message like in the data modules. Make a reference to self to also give hte user the name of the model
  • make the new modules private
  • I would suggest to use v2 as an abbreviation, i.e., _base_model_v2 and _tft_v2.

@phoeenniixx
Copy link
Member Author

Hi @fkiraly, I think this PR is ready to merge

@fkiraly fkiraly merged commit 5d57319 into sktime:main May 22, 2025
35 checks passed
@github-project-automation github-project-automation bot moved this from PR under review to Done in Dec 2024 - Mar 2025 mentee projects May 22, 2025
@github-project-automation github-project-automation bot moved this from PR under review to Done in May - Sep 2025 mentee projects May 22, 2025
@phoeenniixx phoeenniixx deleted the refactor-model branch May 22, 2025 16:50
fkiraly pushed a commit that referenced this pull request Jun 7, 2025
…1813)

### Description

This PR adds example notebook for the new v2 data pipeline vignette,
having the basic implementation of the tft model using this version. For
more info see #1812 , #1811
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants