Repository navigation
Eval caches include the GPU count and type - #455
Merged
Merged
Conversation
…r slot The dispatched-eval determinant omitted the GPU count, so a result could be reused across GPU counts and discount a gate's charge. The determinant, the baseline cache and the pre-budget lookup now carry the GPU count and the lane's GPU type. A run adopts an eval written under the previous slot name in its own directory, so nothing in flight is re-dispatched.
There was a problem hiding this comment.
Round 1 — reviewed head 9d8df6a1 — reviewer summarizer:hermes/gpt-5.6-terra over coverage+credentials+deployment+general+lifecycle+prose.
terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.
Verdict: 1 blocking, 0 advisory.
1 finding attached to the lines below.
Merged verdict: blocking legacy slot adoption can reuse an in-flight evaluation across a GPU lane type change. No findings rejected; the credentials, deployment, and lifecycle reports are one duplicate finding consolidated at the sharpest location and severity.
…measured once with no extra charge
Merged
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.
The dispatched-eval determinant (
DispatchedMeasurer._det) omitted the GPU count, although the baseline cache checks it and its docstring said the determinant covers it. A benchmark whosegpuschanged, with the same image, command, tree and environment, could reuse a result measured with a different GPU count. The pre-budget lookup could also discount the gate charge on that stale entry. Found while reviewing the accelerators design (#454, phase 0).Compatibility (RELEASING.md; measurement caches):
tests/fixtures/dispatched_pre_gpu_identity/, written by the previous kernel, with a completed result and an in-flight job. Each is exercised through the new code: first pass, an idempotent second pass, a retry after an interruption, and a missing submit marker.Upgrading:line in the CHANGELOG.Built by Codex. I reviewed it and asked for the in-flight adoption and fixtures. Gate: pytest, ruff check, ruff format --check, mypy.