Remove GraniteMoeLinearExperts - #2885
Conversation
|
👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review. Note: This is required to complete the testing suite, please only add the label once the PR is code complete and local testing has been performed. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR removes the GraniteMoeLinearExperts wrapper class and its registry mapping, switches the linear_experts registration import to Llama4LinearExperts, removes the Granite-specific skip branch in reap utils' get_moe_attrs, and updates tests to use *Experts classes for Granite and related models, consolidating the standalone Granite test into the parametrized suite. ChangesGranite MoE Linear Experts Removal
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped 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. Comment |
There was a problem hiding this comment.
Code Review
This pull request removes the GraniteMoeLinearExperts implementation and cleans up its references across the codebase, including the specialized test test_linearize_moe_granite. It also updates several MoE model imports in tests (replacing NaiveMoe and ParallelExperts with Experts variants) and adjusts test parameters. A review comment points out a grammatical typo in an error message in reap/utils.py and suggests a correction.
where does this actually happen? or is it just changed in the test? |
457caaf to
7ff0c6d
Compare
|
The quality checks have failed. Please run |
7ff0c6d to
dcbceea
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 Require one maintainer reviewAll PRs must have at least one approving review from a maintainer before merging.
|
dcbceea to
e83ca7a
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e83ca7a to
78b3249
Compare
brian-dellabetta
left a comment
There was a problem hiding this comment.
Have you confirmed the examples still work with these changes?
- examples/quantizing_moe/granite_moe_example.py
- examples/quantization_w8a8_fp8/granite4_example.py
|
@brian-dellabetta I did e2e testing about a month ago when I opened this, but I feel confident that it still works since granitemoe is now in our regular parameterized unit tests |
Purpose
Changes
DeepseekV3NaiveMoe,Glm4MoeNaiveMoe,Glm4MoeLiteNaiveMoe,GlmMoeDsaNaiveMoe, andGraniteMoeParallelExpertsTesting
test_linearize_moepasses