Fix: FlowState h=1 crash due to over-squeeze in _predict_batch#277
Fix: FlowState h=1 crash due to over-squeeze in _predict_batch#277Kushagra7777 wants to merge 4 commits intomainfrom
Conversation
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.
AzulGarza
left a comment
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
let's remove this commented line.
There was a problem hiding this comment.
it seems we are not using this file. it would be best if we remove it.
This PR fixes a shape-related bug where FlowState crashed for
h=1with a singleunique_iddue to an unnecessary.squeeze()collapsing dimensions.What changed
.squeeze()when indexing the0.5quantile.fcst_meankeeps its 2D shape(batch, h)even whenh=1.Result