Skip to content

Changing model_path to dsr1-fp8 for consistency for all STP SGLANg ac…#151

Merged
jgangani merged 2 commits intomainfrom
jgangani_fix_sglang
Feb 6, 2026
Merged

Changing model_path to dsr1-fp8 for consistency for all STP SGLANg ac…#151
jgangani merged 2 commits intomainfrom
jgangani_fix_sglang

Conversation

@jgangani
Copy link
Copy Markdown
Collaborator

@jgangani jgangani commented Feb 6, 2026

…ross GB200 and GB300

Summary by CodeRabbit

  • Chores
    • Updated model references across multiple FP8 recipe configurations for consistency and improved compatibility.
    • Performed minor internal cleanup to remove a redundant import and tidy setup code.

@jgangani jgangani requested a review from ishandhanani February 6, 2026 01:11
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 6, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Updated model.path from "dsfp8" to "dsr1-fp8" across six GB300 FP8 recipe YAMLs and removed a duplicate shutil import by moving a single shutil import in src/srtctl/cli/setup_head.py. No other behavioral changes.

Changes

Cohort / File(s) Summary
GB300 FP8 1k1k Recipes
recipes/gb300-fp8/1k1k/stp/low-latency.yaml, recipes/gb300-fp8/1k1k/stp/max.yaml, recipes/gb300-fp8/1k1k/stp/mid.yaml
Updated model.path from dsfp8 to dsr1-fp8 in each configuration file.
GB300 FP8 8k1k Recipes
recipes/gb300-fp8/8k1k/stp/low-latency.yaml, recipes/gb300-fp8/8k1k/stp/max.yaml, recipes/gb300-fp8/8k1k/stp/mid.yaml
Updated model.path from dsfp8 to dsr1-fp8 in each configuration file.
CLI setup fix
src/srtctl/cli/setup_head.py
Added import shutil at top and removed a duplicate later in the file (dedupe import; no behavioral change).

Sequence Diagram(s)

(omitted — changes do not introduce new multi-component control flow)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • gracehonv
  • trevor-m

Poem

🐰 I hopped through configs, gentle and spry,
Swapped paths from dsfp8 to dsr1-fp8 on the fly,
Tidied an import, no hurry, no haste,
Six files refreshed with a neat little taste,
Happy builds and swift runs — carrots for grace! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: updating model_path references from 'dsfp8' to 'dsr1-fp8' across multiple STP configuration files for consistency.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jgangani_fix_sglang

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@recipes/gb300-fp8/8k1k/stp/max.yaml`:
- Around line 5-6: Several files still reference the old model path "dsfp8"
while this recipe uses "dsr1-fp8"; search the repo for all occurrences of
"dsfp8" (the 27 reported files across H100, GB200-fp8 MTP, and GB300-fp8 MTP
variants) and update them to "dsr1-fp8" where appropriate to match this PR, or
add a clear comment in the specific files that must remain on "dsfp8" explaining
why they differ; ensure you update any YAML keys, model.path references, and
documentation strings referencing "dsfp8" so they consistently use "dsr1-fp8"
(or justify exceptions).

Comment on lines 5 to +6
model:
path: "dsfp8"
path: "dsr1-fp8"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for any remaining references to the old "dsfp8" model path
echo "=== Files still referencing 'dsfp8' ==="
rg -rn 'dsfp8' --type yaml

echo ""
echo "=== All model path values in recipes/ for comparison ==="
rg -n 'path:' --type yaml -g 'recipes/**' | head -50

Repository: ishandhanani/srt-slurm

Length of output: 6155


🏁 Script executed:

#!/bin/bash
# More explicit search for the literal string "dsfp8"
echo "=== Direct literal search for 'dsfp8' ==="
rg 'dsfp8' --type yaml

echo ""
echo "=== GB300 fp8 recipes with STP variant ==="
fd . recipes/gb300-fp8 -type f -name "*.yaml" | grep stp

echo ""
echo "=== GB200 fp8 recipes with STP variant ==="
fd . recipes/gb200-fp8 -type f -name "*.yaml" | grep stp

Repository: ishandhanani/srt-slurm

Length of output: 2205


Address remaining dsfp8 references across the codebase.

The PR changes GB300-fp8 STP variants to use "dsr1-fp8", but 27 files still reference the old "dsfp8" path: 13 H100 files, 8 GB200-fp8 MTP files, and 6 GB300-fp8 MTP files. To achieve consistency across hardware variants and FP8 recipes as claimed in the PR title, update all stale references or clarify why certain variants should remain on the old path.

🤖 Prompt for AI Agents
In `@recipes/gb300-fp8/8k1k/stp/max.yaml` around lines 5 - 6, Several files still
reference the old model path "dsfp8" while this recipe uses "dsr1-fp8"; search
the repo for all occurrences of "dsfp8" (the 27 reported files across H100,
GB200-fp8 MTP, and GB300-fp8 MTP variants) and update them to "dsr1-fp8" where
appropriate to match this PR, or add a clear comment in the specific files that
must remain on "dsfp8" explaining why they differ; ensure you update any YAML
keys, model.path references, and documentation strings referencing "dsfp8" so
they consistently use "dsr1-fp8" (or justify exceptions).

@jgangani jgangani merged commit bb97392 into main Feb 6, 2026
4 of 5 checks passed
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