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

fix Unable to find data type for weight_name='/encoder/layer.0/attention/output/dense/MatMul_output_0' #1959

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

VTrngNghia
Copy link

@VTrngNghia VTrngNghia commented Jul 16, 2024

What does this PR do?

Adds a keyword argument to allow passing extra_options to ORTQuantizer.quantize()

To fix RuntimeError:

Unable to find data type for weight_name='/encoder/layer.0/attention/output/dense/MatMul_output_0'. shape_inference failed to return a type probably this node is from a different domain or using an input produced by such an operator. This may happen if you quantize a model already quantized. You may use extra_options DefaultTensorType to indicate the default weight type, usually onnx.TensorProto.FLOAT.

Maybe it can be added to AutoQuantizationConfig, but there any many @staticmethod for that, so maybe this quick fix is simpler.

Who can review?

It's very simple. Anyone can review.

@VTrngNghia VTrngNghia changed the title feat(quantization): add extra_options fix Unable to find data type for weight_name='/encoder/layer.0/attention/output/dense/MatMul_output_0' Jul 16, 2024
@severinsimmler
Copy link

+1

Thanks for fixing this @VTrngNghia

@Whadup
Copy link

Whadup commented Dec 4, 2024

Is there any harm in just hard-coding "DefaultTensorType": onnx.TensorProto.FLOAT in the extra_options dict?

@home15c6
Copy link

home15c6 commented Dec 4, 2024

Do you mean hard-coding in your package code? Then no "harm" except every time you setup your environment (say, in Docker container), you'll have to apply that change again.
There are tools to automatically apply local changes (and I'm using them until they merge this PR). They're a bit hassle to setup.

@Whadup
Copy link

Whadup commented Dec 4, 2024

I meant to change the code in this PR to

 "extra_options": {
      "WeightSymmetric": quantization_config.weights_symmetric,
      "ActivationSymmetric": quantization_config.activations_symmetric,
      "EnableSubgraph": has_subgraphs,
      "ForceSymmetric": quantization_config.activations_symmetric and quantization_config.weights_symmetric,
      "AddQDQPairToWeight": quantization_config.qdq_add_pair_to_weight,
      "DedicatedQDQPair": quantization_config.qdq_dedicated_pair,
      "QDQOpTypePerChannelSupportToAxis": quantization_config.qdq_op_type_per_channel_support_to_axis,
      "DefaultTensorType": onnx.TensorProto.FLOAT
  },
and not pipe through the extra_options keyword argument

@@ -286,6 +287,7 @@ def quantize(
calibration_tensors_range: Optional[Dict[str, Tuple[float, float]]] = None,
use_external_data_format: bool = False,
preprocessor: Optional[QuantizationPreprocessor] = None,
extra_options: Optional[Dict[str, Any]] = {}
Copy link
Member

Choose a reason for hiding this comment

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

Can you set the default to None and then create an empty dict in the method please?
Dict as default value is not good because they are mutable.

@michaelbenayoun
Copy link
Member

cc @echarlaix

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.

5 participants