Skip to content

docs(launcher): scope CLAUDE.md "new model config" section to Megatron-LM PTQ#1547

Open
h-guo18 wants to merge 1 commit into
mainfrom
haoguo/launcher-claudemd-scope
Open

docs(launcher): scope CLAUDE.md "new model config" section to Megatron-LM PTQ#1547
h-guo18 wants to merge 1 commit into
mainfrom
haoguo/launcher-claudemd-scope

Conversation

@h-guo18
Copy link
Copy Markdown
Contributor

@h-guo18 h-guo18 commented May 26, 2026

What does this PR do?

Type of change: docs

The "Adding a New Model Config" section in tools/launcher/CLAUDE.md is written for the Megatron-LM PTQ pipeline specifically — its env vars (MLM_MODEL_CFG, QUANT_CFG) only apply there. But the generic title and step 1's megatron_lm_ptq.yaml filename were being read as universal:

  • CodeRabbit recently flagged a streaming EAGLE3 yaml (PR #1509) for "missing" MLM_MODEL_CFG / QUANT_CFG, citing this section as the authoritative guideline.
  • Empirically, no other tools/launcher/examples/** yaml sets these vars (HF PTQ, HF / offline / streaming EAGLE3, dflash all use different env conventions).

This PR retitles the section to make scope explicit and adds a one-line lead-in pointing other pipelines at sibling yamls as templates.

Before your PR is "Ready for review"

  • Backward compatible: ✅ (doc-only)
  • New PIP dep: N/A
  • New tests: N/A
  • Changelog: N/A

Summary by CodeRabbit

  • Documentation
    • Updated guidance for adding new Megatron-LM PTQ model configurations with clearer instructions on proper placement and how to copy related pipeline configurations.

Review Change Stack

…n-LM PTQ

The section is Megatron-LM PTQ-specific (env vars MLM_MODEL_CFG / QUANT_CFG only
apply to that pipeline), but the generic title was being read as universal —
e.g. by CodeRabbit, which flagged a streaming EAGLE3 yaml for missing those
vars. Retitle and add a lead-in pointing other pipelines to sibling yamls.

Signed-off-by: h-guo18 <67671475+h-guo18@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e50b9f89-4b18-4385-b6a7-ac016ec0dfac

📥 Commits

Reviewing files that changed from the base of the PR and between 6fed3a6 and 909ced4.

📒 Files selected for processing (1)
  • tools/launcher/CLAUDE.md

📝 Walkthrough

Walkthrough

This PR updates the launcher documentation to clarify the process for adding a new Megatron-LM PTQ model configuration. The section heading is scoped from generic "Adding a New Model Config" to specifically "Adding a New Megatron-LM PTQ Model Config," and the instructions now specify the exact YAML location and explain how to derive other pipeline configurations from sibling files.

Changes

Megatron-LM PTQ Configuration Guidance

Layer / File(s) Summary
Megatron-LM PTQ configuration documentation
tools/launcher/CLAUDE.md
Section heading clarified to target Megatron-LM PTQ configs specifically, with guidance on YAML location (examples/<Org>/<Model>/megatron_lm_ptq.yaml) and instruction to base other pipeline configs (HF PTQ, HF/offline/streaming EAGLE3, dflash) on sibling YAML files.

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately summarizes the main change: scoping the CLAUDE.md documentation section to be explicitly about Megatron-LM PTQ, which directly matches the single documentation alteration described in the raw summary.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Security Anti-Patterns ✅ Passed PR only modifies documentation (tools/launcher/CLAUDE.md). No Python code, dependencies, or security anti-patterns detected.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch haoguo/launcher-claudemd-scope

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

@h-guo18 h-guo18 requested a review from ChenhanYu May 26, 2026 22:56
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.86%. Comparing base (45893f7) to head (909ced4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1547      +/-   ##
==========================================
+ Coverage   76.83%   76.86%   +0.03%     
==========================================
  Files         477      477              
  Lines       51954    51954              
==========================================
+ Hits        39917    39934      +17     
+ Misses      12037    12020      -17     
Flag Coverage Δ
regression 15.24% <ø> (+0.11%) ⬆️
unit 52.76% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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