Skip to content

Conversation

@Kushagra7777
Copy link
Collaborator

This PR fixes a shape-related bug where FlowState crashed for h=1 with a single unique_id due to an unnecessary .squeeze() collapsing dimensions.

What changed

  • Removed the final .squeeze() when indexing the 0.5 quantile.
  • Ensures fcst_mean keeps its 2D shape (batch, h) even when h=1.

Result

  • FlowState now returns stable, consistent output shapes.
  • No more dimension mismatch errors during forecasting.

I found no bug. May be the issue #242mentioned by the user is very specific to their dataset.
Check out the notebook `forecaster-quickstart.ipynb` last cell. h=1 works fine.
removed extra squeeze in _predict_batch so flowstate returns 2D arrays for h=1 with single unique_id.
Copy link
Member

@AzulGarza AzulGarza left a comment

Choose a reason for hiding this comment

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

thanks @Kushagra7777! i see that you're testing that the models does not break when using h=1 in the forecaster-quickstart.ipynb notebook. although that's a strong approach, it would be better if we can add or modify our suite of tests to consider that case.

while reviewing the pr i took a look at our current tests, and it seems that we are already testing Flowstate with h=1 for many frequencies. it would be great if you could take a deeper look to see what might be happening and why we are not catching the case reported by the user.

).prediction_outputs
fcst = fcst.squeeze(-1).transpose(-1, -2) # now shape is (batch, h, quantiles)
fcst_mean = fcst[..., supported_quantiles.index(0.5)].squeeze()
# fcst_mean = fcst[..., supported_quantiles.index(0.5)].squeeze()
Copy link
Member

Choose a reason for hiding this comment

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

let's remove this commented line.

Copy link
Member

Choose a reason for hiding this comment

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

it seems we are not using this file. it would be best if we remove it.

Copy link
Member

Choose a reason for hiding this comment

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

nice

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