-
Notifications
You must be signed in to change notification settings - Fork 200
[MoE] Add conditional expert calibration #1701
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
from pydantic import BaseModel, model_validator | ||
|
||
|
||
class CalibrationConfig(BaseModel): | ||
moe_calibrate_all_experts: bool | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add more information in this config class around what these flags do for future readers, so it's clear which flag should be set for which mode? I was thinking something like:
|
||
moe_calibrate_gated_acts: bool | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Consider renaming to something like |
||
|
||
@model_validator(mode="after") | ||
def validate_config(self): | ||
|
||
if not self.moe_calibrate_gated_acts and not self.moe_calibrate_all_experts: | ||
raise NotImplementedError( | ||
"At least one of moe_calibrate_gated_acts or " | ||
"moe_calibrate_all_experts must be set to True." | ||
) | ||
|
||
return self |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
from unittest.mock import MagicMock | ||
|
||
import pytest | ||
|
||
from llmcompressor.modeling.prepare import replace_modules_for_calibration | ||
|
||
|
||
def test_calib_config(): | ||
model = MagicMock() | ||
with pytest.raises(NotImplementedError) as exc_info: | ||
replace_modules_for_calibration(model, False, False) | ||
|
||
assert str(exc_info.value) == ( | ||
"At least one of moe_calibrate_gated_acts or " | ||
"moe_calibrate_all_experts must be set to True." | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: What do you think about renaming to MoECalibrationConfig?