Skip to content

Add warning for non-divisible group quantization #1401

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

kylesayrs
Copy link
Collaborator

@kylesayrs kylesayrs commented Apr 29, 2025

Purpose

Prerequisites

Changes

  • Added test_observers_update in tests/llmcompressor/modifiers/calibration/test_observers.py
  • Add a warning for attempts to quantize indivisible groups
Attempting to quantize a module weight whose columns (3420) are not divisible by group_size (128). This scheme is not supported by vLLM, please consider adjusting the group_size for modules with this number of columns

Testing

  • This test fails without CT changes, but succeeds with them

Signed-off-by: Kyle Sayers <[email protected]>
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 kylesayrs changed the title [Testing] add observers test [Testing] add failing observers test Apr 29, 2025
@kylesayrs kylesayrs marked this pull request as ready for review May 2, 2025 17:52
@kylesayrs kylesayrs changed the title [Testing] add failing observers test Add warning for non-divisible group quantization May 2, 2025
Copy link
Collaborator

@markurtz markurtz left a comment

Choose a reason for hiding this comment

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

I think we should simplify this using the built-in filter pathways for the logger.add function from Loguru. This way, all log levels and standard functions are supported, and the root logger import will work without other special/additional imports.

Something along the lines of the following:

_logged_once = set()

def deduplicate_filter(record):
    message = record["message"]
    log_once = record["extra"].get("log_once", False)

    if log_once and message in _logged_once:
        return True
    
    if log_once:
        _logged_once.add(message)

    return False
    
logger.add(..., filter=deduplicate_filter)

logger.bind(log_once=True).info("This will only log once")
logger.bind(log_once=True).info("This will only log once")

Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
@kylesayrs kylesayrs changed the base branch from main to kylesayrs/log-once-filter May 14, 2025 14:22
@kylesayrs kylesayrs requested a review from markurtz May 14, 2025 14:25
@kylesayrs kylesayrs dismissed markurtz’s stale review May 14, 2025 14:26

Support log once #1431

Copy link
Collaborator

@brian-dellabetta brian-dellabetta left a comment

Choose a reason for hiding this comment

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

🔥

Base automatically changed from kylesayrs/log-once-filter to main May 14, 2025 21:00
@kylesayrs kylesayrs dismissed brian-dellabetta’s stale review May 14, 2025 21:00

The base branch was changed.

@kylesayrs kylesayrs added the ready When a PR is ready for review label May 19, 2025
Copy link
Collaborator

@brian-dellabetta brian-dellabetta left a comment

Choose a reason for hiding this comment

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

awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready When a PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants