Repository navigation
fix(timesfm3/torch): mask covariate placeholders in mixed batches - #546
Open
rudycelekli wants to merge 1 commit into
Open
rudycelekli wants to merge 1 commit into
rudycelekli wants to merge 1 commit into
Conversation
When only some queries in a predict_batch batch carry past-only or past-future covariates, the others get an all-zero placeholder so the arrays can be stacked. decode was called without past_only_mask / past_future_mask, so the placeholder was read as an observed zero-valued covariate and a no-covariate series' forecast changed depending on which series it was batched with. Build a bool mask per covariate type (True on placeholder rows) and pass it to decode, which already ORs it into the context and horizon masks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's wrong
When only some queries in a
predict_batchbatch havepast_only_covariatesorpast_future_covariates, the others get an all-zero placeholder (np.zeros_like) so the arrays can be stacked.model.decodeis then called withoutpast_only_mask/past_future_mask, so it reads the placeholder as an observed covariate that is constantly zero. As a result, a series with no covariates gets a different forecast depending on which series share its batch.Repro (small random-weight TimesFM3Torch,
per_core_batch_size=2, both contexts 64 long so padding is identical):Before: 6.39. After: ~1e-5 (float noise).
past_only_covariates=[None, po_b]behaves the same way.Fix
For each covariate type present in the batch, build a bool mask with the same shape as the covariate arrays: False for real rows, True for placeholder rows. Pass it to
decodeaspast_only_mask/past_future_mask.decodealready pads these masks and ORs them into the context mask, and for past-future covariates into the horizon mask too. When a covariate type is absent, its mask staysNone, as before. When every row is real, the mask is all False, which is the same asdecode's default.The two fake models in the test file now accept, and ignore, the new keyword arguments.
Testing
TimesFM3ForecasterCovariatePlaceholderTest.test_no_covariate_query_unaffected_by_batch_matechecks that a series' forecast alone matches its forecast when batched with a covariate-bearing series of the same length. It covers both past-future and past-only covariates and uses a tiny random-weightTimesFM3Torchloaded throughfrom_pretrained. Without the fix, both subtests fail (max abs diff about 0.095 against a 1e-4 tolerance). With the fix, both pass.pytest src/timesfm3/torch tests: 109 passed.tests/test_model_loading.pywas excluded because my local environment is missingeinshape.