Skip to content

Conversation

jbachurski
Copy link
Member

@jbachurski jbachurski commented May 1, 2023

Description

  1. Fixes an ungracefully unhandled type case (in materialising a symbolic dimension), which always caused errors when a map-typed operator was used.
  2. Completes ZipMap type inference to properly type the map value types as scalar float tensors.
  3. Adds a test for running ZipMap type inference (and hence testing that maps now are in fact supported by shape inference).

Motivation and Context

ONNX support for the map type is poor and the least documented. This is a small step towards adding more operators using map for more advanced pre-processing capabilities, more dynamic than what is allowed by the existing, attribute-oriented, ai.onnx.ml operators (like LabelEncoder).

Currently, any and all uses of shape inference where a map type is present results in the following error (e.g. for ZipMap):

onnx.onnx_cpp2py_export.shape_inference.InferenceError: [ShapeInferenceError] (op_type:ZipMap): [ShapeInferenceError] type case unsupported for symbolic shape inference. inferred=5

This is due to a piece of code unrelated to a specific inference implementation, and hence map operators cannot be used at all as they will always raise this error. The shape inference test module had a similar lack of a map case. I skimmed through the code and I didn't see other such cases.

Also, related comment where I uncovered this - skl2onnx-produced classifiers use ZipMap by default and hence can never succeed in shape inference.

@jbachurski jbachurski requested review from a team as code owners May 1, 2023 23:00
jbachurski added 6 commits May 2, 2023 00:02
Signed-off-by: jbachurski <kbachurski@gmail.com>
Signed-off-by: jbachurski <kbachurski@gmail.com>
Signed-off-by: jbachurski <kbachurski@gmail.com>
Signed-off-by: jbachurski <kbachurski@gmail.com>
Signed-off-by: jbachurski <kbachurski@gmail.com>
Signed-off-by: jbachurski <kbachurski@gmail.com>
@jbachurski jbachurski force-pushed the fix-map-type-inference branch from 4dc49a2 to 7abb97a Compare May 1, 2023 23:02
jbachurski added 2 commits May 2, 2023 18:02
Signed-off-by: jbachurski <kbachurski@gmail.com>
Signed-off-by: jbachurski <kbachurski@gmail.com>
@jbachurski jbachurski requested a review from xadupre May 2, 2023 19:42
Signed-off-by: jbachurski <kbachurski@gmail.com>
@jbachurski jbachurski force-pushed the fix-map-type-inference branch from 744ffc8 to 4903f5d Compare May 3, 2023 19:33
@jbachurski jbachurski requested a review from gramalingam May 3, 2023 19:34
Signed-off-by: jbachurski <kbachurski@gmail.com>
@jbachurski jbachurski force-pushed the fix-map-type-inference branch from c11161b to 644ccb1 Compare May 3, 2023 19:35
jbachurski and others added 4 commits May 4, 2023 14:10
@gramalingam gramalingam enabled auto-merge (squash) May 4, 2023 21:10
@gramalingam gramalingam merged commit 1b9d067 into onnx:main May 4, 2023
adityagoel4512 pushed a commit to adityagoel4512/onnx that referenced this pull request May 5, 2023
Signed-off-by: jbachurski <kbachurski@gmail.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
Signed-off-by: Aditya Goel <agoel4512@gmail.com>
adityagoel4512 pushed a commit to adityagoel4512/onnx that referenced this pull request May 9, 2023
Signed-off-by: jbachurski <kbachurski@gmail.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
Signed-off-by: Aditya Goel <agoel4512@gmail.com>
adityagoel4512 pushed a commit to adityagoel4512/onnx that referenced this pull request Jul 28, 2023
Signed-off-by: jbachurski <kbachurski@gmail.com>
Co-authored-by: G. Ramalingam <grama@microsoft.com>
Signed-off-by: Aditya Goel <agoel4512@gmail.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.

3 participants