Skip to content

[BUG] Standardize output format of NBeats and NBeatsKAN estimators - #1977

Merged
phoeenniixx merged 5 commits into
sktime:mainfrom
sanskarmodi8:enh/1975-nbeats-nbeatskan-output-format
Oct 8, 2025
Merged

[BUG] Standardize output format of NBeats and NBeatsKAN estimators#1977
phoeenniixx merged 5 commits into
sktime:mainfrom
sanskarmodi8:enh/1975-nbeats-nbeatskan-output-format

Conversation

@sanskarmodi8

@sanskarmodi8 sanskarmodi8 commented Oct 1, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR updates the NBeatsAdapter to ensure that both NBeats and NBeatsKAN models return 3D tensors (batch_size, prediction_length, 1) for point predictions, aligning with the expected output format #1975.

@codecov

codecov Bot commented Oct 1, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@b75ea42). Learn more about missing BASE report.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1977   +/-   ##
=======================================
  Coverage        ?   86.77%           
=======================================
  Files           ?      160           
  Lines           ?     9475           
  Branches        ?        0           
=======================================
  Hits            ?     8222           
  Misses          ?     1253           
  Partials        ?        0
Flag Coverage Δ
cpu 86.77% <ø> (?)
pytest 86.77% <ø> (?)

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.

@sanskarmodi8

sanskarmodi8 commented Oct 1, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I'm not sure why this particular test is failing in CI. When I run pytest -k "test_integration and NHiTS" -v locally, it passes without any errors.

@PranavBhatP

PranavBhatP commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

The tests are passing because it is currently skipped for NBeats and NBeatsKAN. Go to their respective package files namely NBeats_pkg and NBeatsKAN_pkg and remove the test skip tag at the bottom of _tags.

@PranavBhatP

PranavBhatP commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

When I run pytest -k "test_integration and NHiTS" -v

The model we are concerned about is NBeats not NHiTS right...

@sanskarmodi8

sanskarmodi8 commented Oct 1, 2025

Copy link
Copy Markdown
Contributor Author

When I run pytest -k "test_integration and NHiTS" -v

The model we are concerned about is NBeats not NHiTS right...

Ah yes, you’re right—my bad! The test I ran locally was for NHiTS, but the model in my PR is NBeats. I just wanted to check that the changes I made didn’t break the workflow for a similar model.

@PranavBhatP

PranavBhatP commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

Thanks for the changes!

@fkiraly fkiraly added the bug Something isn't working label Oct 1, 2025
@sanskarmodi8

sanskarmodi8 commented Oct 4, 2025

Copy link
Copy Markdown
Contributor Author

Hey, just wanted to check if there’s anything else needed from my side before it can be merged. Thanks!

phoeenniixx
phoeenniixx approved these changes Oct 4, 2025
View reviewed changes

@phoeenniixx phoeenniixx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@phoeenniixx
phoeenniixx merged commit fd849cc into sktime:main Oct 8, 2025
14 of 15 checks passed
@phoeenniixx phoeenniixx changed the title Standardize output format of NBeats and NBeatsKAN estimators [BUG] Standardize output format of NBeats and NBeatsKAN estimators Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phoeenniixx phoeenniixx phoeenniixx approved these changes
@benHeid benHeid Awaiting requested review from benHeid benHeid is a code owner
@fkiraly fkiraly Awaiting requested review from fkiraly fkiraly is a code owner
@fnhirwa fnhirwa Awaiting requested review from fnhirwa
@jdb78 jdb78 Awaiting requested review from jdb78 jdb78 is a code owner
@PranavBhatP PranavBhatP Awaiting requested review from PranavBhatP
@yarnabrina yarnabrina Awaiting requested review from yarnabrina

Assignees

No one assigned

Labels

bug Something isn't working

Projects

Bugfixing - pytorch-forecasting
Status: Fixed/resolved

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sanskarmodi8 @PranavBhatP @phoeenniixx @fkiraly