Skip to content

Keep missing CSV observations on the shared time grid - #520

Open
ayoubchikri wants to merge 2 commits into
google-research:masterfrom
ayoubchikri:fix/csv-time-grid
Open

ayoubchikri wants to merge 2 commits into
google-research:masterfrom
ayoubchikri:fix/csv-time-grid

Conversation

@ayoubchikri

@ayoubchikri ayoubchikri commented Sep 16, 2026 •

Copy link
Copy Markdown

The CSV forecasting example calls dropna() separately for each value column. With missing observations at different dates, that compresses each series independently and loses their shared time grid before TimesFM 2.5 interpolates missing values. Pass each column with its NaNs preserved so preprocessing sees the correct positions.

Reject a column with no observed values before calling model.forecast, with an error naming the column. TimesFM 2.5 cannot forecast such a series, while columns containing some observations still retain their original date positions.

The regression test uses the real TimesFM 2.5 forecast() preprocessing with only decoding mocked. It checks the inputs, interpolated values, padding masks, unchanged DataFrame, and exported forecast dates. A separate test verifies that an all-missing column is rejected before the model is called. The two CSV tests and 16 existing preprocessing-utility tests pass. Ruff passes for the new test; the existing example script has unrelated pre-existing Ruff findings.

@sylvesterkaczmarek sylvesterkaczmarek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The new regression includes an all-missing empty column, but because model.forecast is mocked it never exercises TimesFM's preprocessing. A real TimesFM 2.5 forecast() still fails on an all-NaN series: strip_leading_nans leaves the non-empty all-NaN array unchanged, then linear_interpolation reaches the empty-sample path and raises. So a CSV with an entirely missing value column still crashes even though this test passes. Please either reject/skip all-missing columns before calling the model or make preprocessing support them, and exercise this case through the real preprocessing path.

@ayoubchikri

Copy link
Copy Markdown
Author

Thanks for spotting this. I now reject an all-missing CSV column before calling model.forecast, with an error naming the column. The test runs the real TimesFM 2.5 forecast() preprocessing for columns with observations, with only decoding mocked, and verifies that an all-missing column is rejected before the model is called. All 18 targeted tests pass.

@sylvesterkaczmarek sylvesterkaczmarek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rechecked 26339d2. All-missing CSV columns are now rejected before model.forecast, and the regression runs the real TimesFM 2.5 preprocessing path for valid columns while asserting the all-missing column never reaches the model. This resolves the preprocessing crash I raised.

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