Bake SciCode sandbox image once; pre-build from launch host#159
Bake SciCode sandbox image once; pre-build from launch host#159finbarrtimbers wants to merge 5 commits into
Conversation
…r submit Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…de Opus 4.7 <noreply@anthropic.com>
…Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fa3685c6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| f"RUN mkdir -p {deps_dir}", | ||
| f"RUN echo {pyproject_b64} | base64 -d > {deps_dir}/pyproject.toml", | ||
| f"RUN echo {uvlock_b64} | base64 -d > {deps_dir}/uv.lock", | ||
| f"RUN uv sync --active --frozen --no-dev --project {deps_dir}", |
There was a problem hiding this comment.
Install SciCode deps into the active sandbox venv
This uv sync --active command now runs during image build, but in get_swerex_image the injected dockerfile_extra lines execute before VIRTUAL_ENV/PATH are set to /root/venv (src/olmo_eval/harness/sandbox/image.py). In that context, uv creates a project-local .venv under /opt/scicode-deps instead of installing into the runtime venv used by sandbox execution, so SciCode verification can hit missing numpy/scipy/sympy/h5py imports after the runtime uv sync step was removed.
Useful? React with 👍 / 👎.
| base_image, | ||
| container_runtime=runtime, | ||
| dockerfile_extra=dockerfile_extra, | ||
| require_registry=True, |
There was a problem hiding this comment.
Avoid making sandbox pre-bake require registry push access
Calling get_swerex_image(..., require_registry=True) makes pre-bake a hard prerequisite for launch: if the launch host lacks push auth to SWEREX_REGISTRY, this raises and aborts beaker launch before any job is submitted. That is a regression from prior behavior where the Beaker job could still build and run the derived image locally on the GPU node (with require_registry=False) even when registry push failed.
Useful? React with 👍 / 👎.
…cycle Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
0fa3685 to
082fe30
Compare
Stacks on #153 and #160 — both must merge first.
Previously, every SciCode sub-step launch ran
uv syncinside the sandbox to installnumpy/scipy/sympy/h5pyfrom a mounted lockfile, blocking the GPU on hundreds of redundant resolves. This PR bakes the deps into the sandbox image once and pre-builds it on the launch host before submitting the Beaker job, so the GPU node only has to pull a ready image.Mechanism
SciCodeExternalEval.prebuild_sandbox_specs()declares its lockfile asdockerfile_extralines on the swerex base image. Thedockerfile_extraplumbing is already hashed intoget_swerex_image's deterministic tag (src/olmo_eval/harness/sandbox/image.py:55-57), so dep changes invalidate the cache automatically._prebuild_external_eval_sandbox_imagesbefore submitting (src/olmo_eval/cli/beaker/launch.py), callingget_swerex_image(..., require_registry=True)on the user's machine. The launch host always pushes toBEAKER_INFRA_ENV_VARS["SWEREX_REGISTRY"]— the same registry the Beaker job pulls from — so users don't have to setSWEREX_REGISTRYin their shell.ExternalEval.prebuild_sandbox_specs()defaults to returning(), so non-SciCode external evals are no-op pass-throughs.Measurements
max_concurrency × n_problems.Circular-import dependency
The new top-level
from olmo_eval.harness.sandbox import SandboxManagerinscicode/eval.pytriggers a cycle throughrunners.io.storage → cli.utils → cli/__init__ → harness.presets. The fix lives in #160 (move sharedconsoletoolmo_eval.common.console) — split out for independent review. Once #160 merges, this branch should be rebased to drop its duplicate of that commit.Verification
uv run pytest tests/evals/external/benchmarks/test_scicode_external.py tests/cli— 54/54 pass, including a newTestPrebuildSandboxSpecs.make lintclean.