Skip to content

fix(pytorch): Rename layer_scale parameter to avoid quantization error #2172

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

Merged
merged 18 commits into from
Apr 23, 2025

Conversation

ved1beta
Copy link
Contributor

Here's the formatted PR description based on our changes:

Type of Change

  • Bug fix
  • No API changes (parameter rename only)

Description

This PR fixes an issue where PyTorch models using layer_scale parameters fail during quantization due to tensor conversion errors. The problem occurs when the quantization process attempts to convert tensor-type scale parameters to Python scalars.

Key changes:

  1. Renamed problematic layer_scale parameter to layer_gamma to avoid conflicts with scale detection
  2. Enhanced _get_module_scale_zeropoint method to better handle scale parameters
  3. Added improved logging for scale parameter detection

The fix maintains backward compatibility while resolving the quantization failure.

Expected Behavior & Potential Risk

Expected Behavior:

  • Models using the renamed layer_gamma parameter will successfully complete quantization
  • Existing scale detection logic works correctly with the new parameter name
  • Debug logs provide better visibility into scale parameter detection

Potential Risks:

  • Models explicitly looking for layer_scale parameter name might need updates
  • Need to communicate the parameter rename in documentation
  • May need to provide migration guide for existing models

How has this PR been tested?

Tests have been implemented in test/adaptor/test_pytorch_layer_scale.py with two test cases:

  1. test_layer_scale_error: Verifies the original issue by confirming that models with layer_scale parameter fail quantization with the expected error
  2. test_layer_gamma_success: Validates that models using the new layer_gamma parameter successfully complete quantization

Test Environment:

  • Python 3
  • PyTorch latest version
  • Neural Compressor latest version
  • Test models: ConvEncoder architecture with both original and fixed parameter names
  • Input tensor shape: (1, 64, 32, 32)

To reproduce:

cd neural-compressor
python3 test/adaptor/test_pytorch_layer_scale.py

Dependency Change?

No new dependencies were introduced or removed.

  • Uses existing PyTorch and Neural Compressor infrastructure
  • All changes are internal to the codebase

@thuang6 thuang6 requested review from xin3he, XuehaoSun and Copilot and removed request for xin3he April 14, 2025 14:13
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

neural_compressor/adaptor/pytorch.py:4175

  • The current filter condition excludes any parameter containing 'gamma', which might unintentionally filter out valid parameters. Consider checking explicitly for 'layer_scale' and 'layer_gamma' instead to avoid potential misclassification.
if "scale" in node.target and not any(exclude in node.target for exclude in ["layer_scale", "gamma"]):

@xin3he
Copy link
Contributor

xin3he commented Apr 15, 2025

Hi @ved1beta Thanks for your commit.
I notice that the unexpected case is that scale is a tensor list, not one value. If so, we can fix it in a better way.

Suggestion:
float(getattr(model, node.target)) -> getattr(model, node.target).tolist() # it can cover both one value and list value

BTW, currently our CI is blocked by #2171. To resolve the CI issue, please update your branch after #2171 is merged.

pre-commit-ci bot and others added 4 commits April 17, 2025 10:01
* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/pycqa/isort: 5.13.2 → 6.0.1](PyCQA/isort@5.13.2...6.0.1)
- [github.com/psf/black.git: 24.10.0 → 25.1.0](https://github.com/psf/black.git/compare/24.10.0...25.1.0)
- [github.com/codespell-project/codespell: v2.3.0 → v2.4.1](codespell-project/codespell@v2.3.0...v2.4.1)
- [github.com/astral-sh/ruff-pre-commit: v0.8.6 → v0.11.4](astral-sh/ruff-pre-commit@v0.8.6...v0.11.4)

Signed-off-by: Sun, Xuehao <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: changwangss <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: xin3he <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@ved1beta
Copy link
Contributor Author

hey i did required changes but had a merge conflict , fixing it

@xin3he xin3he merged commit f0812a1 into intel:master Apr 23, 2025
31 checks passed
@xin3he
Copy link
Contributor

xin3he commented Apr 23, 2025

Thanks, @ved1beta, merged

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