Skip to content

[wip] Kylel/debug olmo core eval#181

Open
kyleclo wants to merge 5 commits intomainfrom
kylel/debug-olmo-core-eval
Open

[wip] Kylel/debug olmo core eval#181
kyleclo wants to merge 5 commits intomainfrom
kylel/debug-olmo-core-eval

Conversation

@kyleclo
Copy link
Collaborator

@kyleclo kyleclo commented Dec 18, 2025

there were some bugs in launching a command of this pattern (custom oe eval branch, using gantry)

olmo-cookbook-eval evaluate "$CHECKPOINT_PATH" \
    --tasks "$TASK" \
    --cluster "$CLUSTER" \
    --num-gpus 2 \
    --model-backend vllm \
    --partition-size 8 \
    --use-hf-token \
    --dashboard "$DASHBOARD" \
    --workspace "$WORKSPACE" \
    --vllm-use-v1-spec \
    --vllm-memory-utilization=0.8 \
    --model-args max_length=4096,trust_remote_code=true \
    --use-gantry \
    --oe-eval-branch "$OE_EVAL_BRANCH" \
    $DRY_RUN_FLAG

this change handles tracking of that git branch (and handles a sub-case if the custom branch specified is 'main')

# Check if we're already on the target branch (e.g., main after shallow clone)
result = subprocess.run(
["git", "rev-parse", "--abbrev-ref", "HEAD"], capture_output=True, text=True
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing error check on subprocess git command

The subprocess.run call for git rev-parse --abbrev-ref HEAD is missing check=True, unlike all other subprocess calls in this function. If this git command fails, the code continues silently with result.stdout potentially empty or containing unexpected output. The subsequent comparison current_branch == commit_branch could then incorrectly fall through to the else branch, masking the actual error.

Fix in Cursor Fix in Web

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