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

Support layerwise quantization #1018

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

Conversation

changwangss
Copy link
Contributor

@changwangss changwangss commented Nov 22, 2024

What does this PR do?

INC support layer-wise feature, both supported cpu and xpu.
because 3.2 plan to release Dec, 9, so I limit the INC installation commit now.
Once INC 3.2 is officially released, I will raise a PR to update this

Fixes # (issue)

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Comment on lines 403 to 406
model = load_empty_model(
model_id,
trust_remote_code=trust_remote_code,
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The model was already loaded above so would make sense to remove this part as well, alos we need to set cls=load_empty_model when loading the model with load_empty_model https://github.com/intel/neural-compressor/blob/v3.1.1/neural_compressor/torch/utils/utility.py#L354

Suggested change
model = load_empty_model(
model_id,
trust_remote_code=trust_remote_code,
)
model = load_empty_model(model_id, cls=model_class, **loading_kwargs)

Copy link
Contributor Author

@changwangss changwangss Nov 26, 2024

Choose a reason for hiding this comment

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

I improve the code, load_empty_model only needed by "layer-wise" feature.
I didn't pass loading_kwargs, because load_empty_model function doesn't support loading_kwargs, the error raised.

>           model = cls(config, **kwargs)
E           TypeError: __init__() got an unexpected keyword argument 'subfolder'

)
else:
quantization_config = RtnConfig(bits=bits, group_size=8)
quantization_config = RtnConfig(bits=bits, group_size=8, use_layer_wise=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to specify it when creating the quantization config ? Looks like with the current integration this information will be ignored (load_empty_model will be called in all cases)

Copy link
Contributor Author

@changwangss changwangss Nov 26, 2024

Choose a reason for hiding this comment

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

I improve it, add a case to test layer-wise.

Signed-off-by: changwangss <chang1.wang@intel.com>
Signed-off-by: changwa1 <chang1.wang@intel.com>
Signed-off-by: changwa1 <chang1.wang@intel.com>
Signed-off-by: changwa1 <chang1.wang@intel.com>
Signed-off-by: changwa1 <chang1.wang@intel.com>
Signed-off-by: changwa1 <chang1.wang@intel.com>
@changwangss
Copy link
Contributor Author

Hi @echarlaix,

Due to some layer-wise bug fixes in INC 3.2, and with the release planned for 12.9, I’ve set the installation to point to a specific commit for now. Once INC 3.2 is officially released, I will raise a PR to update this. Let me know your thoughts!

@changwangss
Copy link
Contributor Author

CI IPEX issue fixed by #1009 , detail has discussed with @IlyasMoutawwakil in #1027 .

optimum/intel/neural_compressor/quantization.py Outdated Show resolved Hide resolved
if hasattr(quantization_config, "use_layer_wise") and quantization_config.use_layer_wise:
from neural_compressor.torch import load_empty_model

model = load_empty_model(model_id, cls=model_class, trust_remote_code=trust_remote_code)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not :

Suggested change
model = load_empty_model(model_id, cls=model_class, trust_remote_code=trust_remote_code)
model = load_empty_model(model_id, cls=model_class, **loading_kwargs)

Copy link
Contributor Author

@changwangss changwangss Nov 29, 2024

Choose a reason for hiding this comment

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

Let me make explaination, because we initialize the config first, and then initialize the model in load_empty_model function. https://github.com/intel/neural-compressor/blob/e2696603f45f5796f1c048aab33eef11aaeb2cdb/neural_compressor/torch/utils/utility.py#L356

when the **loading_kwargs passed, config initialization will raise error like the following.

>           model = cls(config, **kwargs)
E           TypeError: __init__() got an unexpected keyword argument 'subfolder'

if hasattr(quantization_config, "use_layer_wise") and quantization_config.use_layer_wise:
from neural_compressor.torch import load_empty_model

model = load_empty_model(model_id, cls=model_class, trust_remote_code=trust_remote_code)
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks the same for both cpu / xpu, shouldn't it be moved to the correct device ? also should be moved outside of the if use_xpu condition as the code is duplicated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree, I improved it.

changwangss and others added 4 commits November 29, 2024 17:27
Co-authored-by: Ella Charlaix <80481427+echarlaix@users.noreply.github.com>
Signed-off-by: sys-lpot-val <sys_lpot_val@intel.com>
Signed-off-by: changwangss <chang1.wang@intel.com>
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.

4 participants