Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: SparseGPT recipes #1142

Merged
merged 4 commits into from
Feb 12, 2025
Merged

Update: SparseGPT recipes #1142

merged 4 commits into from
Feb 12, 2025

Conversation

rahul-tuli
Copy link
Collaborator

@rahul-tuli rahul-tuli commented Feb 12, 2025

Issue

The following test was failing:

FAILED tests/e2e/vLLM/test_vllm.py::TestvLLM_0_tests_e2e_vLLM_configs_sparse2of4_fp8_dynamic_yaml::test_vllm  
ValueError: There is no module or parameter named 'lm_head.bitmask' in LlamaForCausalLM

This issue arose due to recent improvements in SparseGPTModifier, which changed its default behavior. Previously, lm_head was silently ignored, but the new updates no longer do so automatically.

Fix

The fix involves explicitly updating the affected recipes to include the parameter:

ignore: ["re:.*lm_head"]

when all layers are targeted. This ensures that lm_head is properly excluded and prevents the failure.

Example Change

Previously, we relied on regex patterns to target linear layers while ignoring lm_head. The updated configuration now explicitly targets linear layers and ignores lm_head:

-    "targets": ["re:model.layers.\\d+$"],
+    "targets": ["Linear"],
+    "ignore": ["re:.*lm_head"]

This provides a more structured approach and avoids unnecessary regex-based filtering.

Additional Fixes & Improvements

  • Removed the deprecated argument sequential_update.
  • Updated recipes to use "targets": ["Linear"] instead of regex matching for better clarity and maintainability.
  • Raise Warning when lm_head is targetted. Contributed by @kylesayrs

Copy link

👋 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.

kylesayrs and others added 2 commits February 12, 2025 18:34
@rahul-tuli rahul-tuli force-pushed the update-ignores-in-sparsegpt-recipes branch from 2f08151 to e8d94b6 Compare February 12, 2025 18:34
@rahul-tuli rahul-tuli self-assigned this Feb 12, 2025
@dsikka
Copy link
Collaborator

dsikka commented Feb 12, 2025

Do we know why the lm_head in the sparse_2of4_only e2e test was being skipped by the sparse24 bitmask compressor?

@dsikka
Copy link
Collaborator

dsikka commented Feb 12, 2025

Is this ready for review? still draft

Signed-off-by: Rahul Tuli <[email protected]>
@rahul-tuli
Copy link
Collaborator Author

Is this ready for review? still draft

No it wasn't, now is!

@rahul-tuli rahul-tuli marked this pull request as ready for review February 12, 2025 19:17
Signed-off-by: Rahul Tuli <[email protected]>
@dsikka dsikka merged commit 98a7ae6 into main Feb 12, 2025
7 checks passed
@dsikka dsikka deleted the update-ignores-in-sparsegpt-recipes branch February 12, 2025 23:43
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.

3 participants