Skip to content

Correct the macOS --no_auto_dataset_cache guidance in AGENTS.md - #1907

Open
VishnuR23 wants to merge 1 commit into
allenai:mainfrom
VishnuR23:vishnu/agents-md-no-auto-dataset-cache
Open

VishnuR23 wants to merge 1 commit into
allenai:mainfrom
VishnuR23:vishnu/agents-md-no-auto-dataset-cache

Conversation

@VishnuR23

Copy link
Copy Markdown

Problem

AGENTS.md tells every mason.py launch script to pass --no_auto_dataset_cache on macOS:

Experiment launch scripts that call mason.py must include --no_auto_dataset_cache (before the -- separator) because vllm is not installed locally on macOS. Without this flag, mason.py tries to cache the dataset locally which fails on the import vllm in data_loader.py.

#1866 removed that import. Its own CHANGELOG entry says so:

... and mason.py no longer needs --no_auto_dataset_cache on macOS for the vllm import

but this line was not updated alongside it, so the agent instructions now contradict the changelog.

What is actually true now

Local caching re-runs the training entrypoint with --cache_dataset_only, so what matters is whether that module imports vllm. Measured on macOS at b926978, importing each of the six entries in OPEN_INSTRUCT_COMMANDS:

entrypoint on macOS
finetune.py imports OK
dpo.py imports OK
dpo_tune_cache.py imports OK
reward_modeling.py imports OK
grpo.py ModuleNotFoundError: No module named 'vllm'
grpo_fast.py ModuleNotFoundError: No module named 'vllm'

So the blanket rule is now wrong for four of six and still right for two. The two that fail do so for reasons unrelated to data_loader.py:

  • grpo_fast.py:80from vllm.distributed.weight_transfer.base import WeightTransferInitRequest
  • grpo.py:34 — imports vllm_utils, which does a plain import vllm at line 39

Change

Scopes the rule to GRPO scripts and states the mechanism, so it stays checkable as more entrypoints drop the import rather than going stale again silently.

It also records something that surprised me while checking: mason.py does not protect you here either way. On macOS it prints a hint and then attempts caching anyway:

skip_caching = args.no_auto_dataset_cache
if sys.platform == "darwin" and not args.no_auto_dataset_cache:
    console.log("[yellow]⚠️  On macOS, consider using --no_auto_dataset_cache "
                "(vllm not available for local caching)[/yellow]")

That message is now over-broad for the same reason the AGENTS.md line was — it fires for finetune.py and dpo.py runs that would cache fine. I left mason.py alone to keep this docs-only; narrowing the warning to the GRPO entrypoints would be a reasonable follow-up if you want it.

Testing

No code changed. The table above is the verification — each entrypoint imported directly under uv run on macOS/arm64.

Docs only: no files under open_instruct/, so the CHANGELOG check does not apply and no GPU code paths are touched.

GPU_TESTS=bypass

🤖 Generated with Claude Code

AGENTS.md told every mason.py launch script to pass
--no_auto_dataset_cache on macOS, because local caching "fails on the
`import vllm` in `data_loader.py`". allenai#1866 removed that import, and its
own CHANGELOG entry says mason no longer needs the flag for it, but this
line was not updated.

The blanket rule is now wrong for most entrypoints and still right for
two. Local caching re-runs the training entrypoint with
--cache_dataset_only, so what matters is whether that module imports
vllm. Measured on macOS at b926978:

    finetune.py         imports OK
    dpo.py              imports OK
    dpo_tune_cache.py   imports OK
    reward_modeling.py  imports OK
    grpo.py             ModuleNotFoundError: No module named 'vllm'
    grpo_fast.py        ModuleNotFoundError: No module named 'vllm'

grpo_fast.py still imports vllm at module level
(vllm.distributed.weight_transfer, line 80) and grpo.py pulls it in
through vllm_utils, which imports vllm at line 39.

Rewrites the line to scope the flag to GRPO scripts and to say why, so
the rule stays checkable as more entrypoints drop the import. Also notes
that mason.py only prints a hint on macOS and still attempts caching, so
it does not protect you either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant