Revert unverified EAGLE3 model examples; keep triage code + baseline - #1623
Conversation
Per review (Hao Guo): main should be production-ready, and most of the EAGLE3 model example YAMLs added in #1417 are not yet verified to work. Land the triage infrastructure now and re-add model examples in follow-up PRs once each is verified end-to-end. Removed: - All unverified per-model launcher configs (hf_offline_eagle3.yaml + eagle3_quick_check.yaml) for DeepSeek-V3.2, GLM-5, MiniMax-M2.5, Ministral-3-8B/14B, Kimi-K2.5 (+NVFP4), GPT-OSS-20B, Qwen3.5-9B/27B/35B-A3B, Step-3.5-Flash. - Per-model status docs (EAGLE3_TRIAGE.md, eagle3_triage_chart.md). Kept: - Verified baseline example Qwen/Qwen3-8B/eagle3_quick_check.yaml. - Triage/infra code: launcher common scripts (vLLM native-extractor dump), compute_hidden_states_vllm.py, and the modelopt fixes (FakeBaseModel VLM detection, consolidated.safetensors load, use_cache export). - New-model triage guide (status tracking pointed to the internal tracker). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ye Yu <yeyu@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (27)
💤 Files with no reviewable changes (26)
📝 WalkthroughWalkthroughThis PR consolidates the EAGLE3 triage workflow by updating contributor guidance to use an internal triage tracker instead of maintaining multiple documentation files, and removes all associated example pipeline configurations and deprecated triage charts. ChangesEAGLE3 Triage Workflow Consolidation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1623 +/- ##
==========================================
+ Coverage 74.57% 74.79% +0.21%
==========================================
Files 482 482
Lines 52960 52960
==========================================
+ Hits 39495 39610 +115
+ Misses 13465 13350 -115
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
There was a problem hiding this comment.
I think this file should not be committed in. @yeyu-nvidia
There was a problem hiding this comment.
Good catch — agreed, it's an ephemeral scheduler lock that slipped in. Removed from tracking + added a .gitignore rule in #1758.
## What Remove `.claude/scheduled_tasks.lock` from version control and add a `.gitignore` rule so it is never committed again. ## Why This file is an **ephemeral Claude Code scheduler lock** — its contents are runtime process state (`sessionId`, `pid`, `procStart`, `acquiredAt`), not source. It was accidentally committed in #1623 and is currently tracked on `main`. Reported by @sychen52 in [review of #1623](#1623 (review)). ## Changes - `git rm --cached .claude/scheduled_tasks.lock` - Add `.claude/scheduled_tasks.lock` to `.gitignore` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated repository configuration to exclude internal runtime lock files from version control. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Ye Yu <yeyu@nvidia.com>
## What Remove `.claude/scheduled_tasks.lock` from version control and add a `.gitignore` rule so it is never committed again. ## Why This file is an **ephemeral Claude Code scheduler lock** — its contents are runtime process state (`sessionId`, `pid`, `procStart`, `acquiredAt`), not source. It was accidentally committed in #1623 and is currently tracked on `main`. Reported by @sychen52 in [review of #1623](#1623 (review)). ## Changes - `git rm --cached .claude/scheduled_tasks.lock` - Add `.claude/scheduled_tasks.lock` to `.gitignore` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated repository configuration to exclude internal runtime lock files from version control. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Ye Yu <yeyu@nvidia.com>
What does this PR do?
Type of change: Revert / cleanup (follow-up to #1417)
Per review feedback (@h-guo18):
mainshould be production-ready and user-facing. Most of the EAGLE3 model example YAMLs added in #1417 are not yet verified to work end-to-end in modelopt (~80% fail at some pipeline stage), which is confusing to ship. The agreed plan is to land the triage infrastructure now and re-add each model's launcher YAML in a dedicated follow-up PR once it is verified green.Removed (unverified, to be re-added per-model once verified):
hf_offline_eagle3.yaml+eagle3_quick_check.yaml) for: DeepSeek-V3.2, GLM-5, MiniMax-M2.5, Ministral-3-8B, Ministral-3-14B, Kimi-K2.5, Kimi-K2.5-NVFP4, GPT-OSS-20B, Qwen3.5-9B, Qwen3.5-27B, Qwen3.5-35B-A3B, Step-3.5-Flash.tools/launcher/examples/EAGLE3_TRIAGE.md,examples/speculative_decoding/pipeline/eagle3/eagle3_triage_chart.md(volatile status — tracked internally instead).Kept (the durable triage infrastructure from #1417):
tools/launcher/examples/Qwen/Qwen3-8B/eagle3_quick_check.yaml.compute_hidden_states_vllm.py.consolidated.safetensorsload,use_cacheexport templates.eagle3_new_model_triage_guide.md); its "document results" step now points at the internal tracker rather than the removed chart.Testing
No code paths change — this only removes example YAMLs and two status docs and edits one doc reference. Pre-commit (ruff/markdownlint/yaml/license) passes on the kept/edited files.
Before your PR is "Ready for review"
CONTRIBUTING.md: N/AAdditional Information
Follow-up to #1417. Next step (tracked separately): verify each removed model end-to-end in modelopt, then re-add its YAML in a dedicated PR. Note: the nmm-sandbox weekly EAGLE3 CI is being trimmed to the Qwen3-8B baseline to match.
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores