Skip to content

feat: add gift-eval exp replication - #4

Merged
AzulGarza merged 27 commits into
mainfrom
feat/gift-eval-rep
Sep 3, 2026
Merged

feat: add gift-eval exp replication#4
AzulGarza merged 27 commits into
mainfrom
feat/gift-eval-rep

Conversation

@AzulGarza

Copy link
Copy Markdown
Member

this pr ads an experiment to replicate gift-eval with foundationforecast

Copilot AI 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.

Pull request overview

This PR adds and documents an experiment workflow to replicate GIFT-Eval results using foundationforecast, including model registry expansion, verification tooling, and job orchestration updates.

Changes:

  • Expand GIFT-Eval experiment model registry (with reference slugs, aliases, and CI subset updates) and add verification + replication-table generation utilities.
  • Add per-job timing capture (timing.json) and backfill/rerun helpers for CI and Modal execution.
  • Adjust model dispatch/validation for TiRex and TimesFM to support GIFT-Eval-specific repo IDs; update docs/examples accordingly.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/models/test_tirex.py Extends TiRex dispatch tests for GIFT-Eval-specific repo IDs.
foundationforecast/models/tirex.py Updates TiRex v2 detection logic to include TiRex-2-* variants.
foundationforecast/models/timesfm.py Adds allowlist to permit specific non-pytorch TimesFM repo IDs for replication runs.
experiments/gift-eval/uv.lock Switches experiment package source to editable.
experiments/gift-eval/src/verify/verify.py Adds model-level result aggregation and verification utilities plus model filtering helpers.
experiments/gift-eval/src/verify/replication_table.py New module to build/write a replication analysis CSV (metrics + diffs + timing).
experiments/gift-eval/src/verify/reference.py Defines shared metric column constants for verification/reporting.
experiments/gift-eval/src/runners/run_verify.py New CLI runner to verify results and generate replication tables (local/CI modes).
experiments/gift-eval/src/runners/run_modal.py Refactors Modal dispatch, adds S3-based selection for missing results/timing, adds force mode.
experiments/gift-eval/src/runners/run_ci.py Adds local-only “missing timing” rerun mode for CI subset jobs.
experiments/gift-eval/src/eval/models.py Ensures per-model kwargs isolation and defaults alias from reference_slug when unset.
experiments/gift-eval/src/eval/jobs.py Adds timing.json path helper and job filter for missing timing.
experiments/gift-eval/src/eval/evaluate.py Records per-job wall time to timing.json and logs elapsed time.
experiments/gift-eval/README.md Documents verification workflow, replication table semantics, and timing backfill strategies.
experiments/gift-eval/Makefile Adds sync/verify/table targets for experiment workflows.
experiments/gift-eval/configs/models.yaml Expands/normalizes model registry (keys, repo_id mapping, reference slugs, alias defaults).
experiments/gift-eval/configs/ci_subset.yaml Updates CI subset model keys to match new registry keys.
docs/examples/ts-foundation-models-comparison-quickstart.ipynb Updates import list (removes Toto from example import).
docs/examples/forecaster-quickstart.ipynb Formatting-only change (blank line).
docs/examples/finetuning.ipynb Reorders/cleans imports and normalizes typography in markdown/output.
docs/examples/anomaly-detection-forecaster-quickstart.ipynb Formatting-only change (blank line).
Suppressed comments (1)

foundationforecast/models/timesfm.py:255

  • The ValueError message is now misleading: this guard allows repo_ids that do not contain "pytorch" (via the allowlist), but the error text still says "only supports pytorch models" and suggests opening an issue for JAX. Please update the message to reflect the actual acceptance criteria / allowlist so users aren’t confused when a non-"pytorch" repo_id is permitted (or rejected).
        if "pytorch" not in repo_id and repo_id not in _GIFT_EVAL_TORCH_REPOS:
            raise ValueError(
                "TimesFM only supports pytorch models, "
                "if you'd like to use jax, please open an issue"
            )

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread foundationforecast/models/timesfm.py Outdated
Comment thread experiments/gift-eval/src/runners/run_modal.py

Copilot AI 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.

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 4 comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

experiments/gift-eval/configs/ci_subset.yaml:1

  • The header comment says this is "one GPU job per wrapper class", but the file currently lists a smaller subset. Please update the comment to match what the file actually contains (or add the missing jobs).
# CI subset: one GPU job per wrapper class (+ chronos on m4_hourly).

foundationforecast/models/timesfm.py:255

  • The repo_id gate now allows non-*pytorch* IDs via _GIFT_EVAL_TORCH_REPOS, but the error message still implies the only valid IDs contain "pytorch". Updating the message to describe the actual acceptance criteria will make failures easier to diagnose.
        if "pytorch" not in repo_id and repo_id not in _GIFT_EVAL_TORCH_REPOS:
            raise ValueError(
                "TimesFM only supports pytorch models, "
                "if you'd like to use jax, please open an issue"
            )

Comment thread foundationforecast/models/patchtst_fm.py Outdated
Comment thread foundationforecast/models/flowstate.py Outdated
Comment thread experiments/gift-eval/README.md Outdated
Comment thread experiments/gift-eval/README.md

Copilot AI 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.

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

foundationforecast/models/timesfm.py:255

  • The ValueError message says to "open an issue" for JAX, but the code now explicitly allowlists a repo_id with a "-jax" suffix. This makes the error misleading for users who hit the guard with other non-pytorch repo_ids.
        if "pytorch" not in repo_id and repo_id not in _GIFT_EVAL_TORCH_REPOS:
            raise ValueError(
                "TimesFM only supports pytorch models, "
                "if you'd like to use jax, please open an issue"
            )

foundationforecast/models/timesfm.py:20

  • _GIFT_EVAL_TORCH_REPOS includes a repo_id with a "-jax" suffix, which makes the name misleading and invites accidental removal/incorrect assumptions. Adding a short comment explaining why this allowlist exists (and why a "-jax"-named repo is still acceptable here) would make the intent clear.
_GIFT_EVAL_TORCH_REPOS = (
    "google/timesfm-1.0-200m",
    "google/timesfm-2.0-500m-jax",
)

experiments/gift-eval/src/verify/verify.py:120

  • verify_model() now compares only MASE/CRPS via compare_results(), but it still fails the run if any column in the actual results contains NaNs. That can cause false failures due to unrelated/secondary columns that are not part of the replication criteria.
    actual = load_actual_results(model_key, output_root)
    if actual.isna().any().any():
        raise AssertionError(f"NaN values found in actual results for {model_key!r}")

Comment thread foundationforecast/models/patchtst_fm.py
Comment thread foundationforecast/models/flowstate.py
@AzulGarza
AzulGarza merged commit 659b3dd into main Sep 3, 2026
8 checks passed
@AzulGarza
AzulGarza deleted the feat/gift-eval-rep branch September 3, 2026 22:41
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