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

Enable Double input type for RandomForest and ZipMap #826

Merged
merged 3 commits into from
Feb 23, 2022

Conversation

BowenBao
Copy link
Collaborator

  • Regardless of input type, attribute types remains Float for floating attributes of RandomForest.
  • Regardless of input type, probabilities output type of RandomForest remains Float.
  • ZipMap only supports Float input & output type. Update shape_calculator to infer output type from input type.

Signed-off-by: BowenBao bowbao@microsoft.com

@xadupre
Copy link
Collaborator

xadupre commented Feb 14, 2022

Until ml opset 1, float attributes were the only supported ones (see https://github.com/onnx/onnx/blob/main/docs/Changelog-ml.md#ai.onnx.ml.TreeEnsembleClassifier-1). It changes in ml opset 3 (https://github.com/onnx/onnx/blob/main/docs/Operators-ml.md#ai.onnx.ml.TreeEnsembleClassifier). This was introduced to fix discrepancies due to the float attributes (see https://onnx.ai/sklearn-onnx/auto_tutorial/plot_ebegin_float_double.html).

@BowenBao
Copy link
Collaborator Author

@xadupre Thanks for info. Hence for opset 3, I only need to update the output type of TreeEnsembleClassifier, and type for ZipMap. Attribute type can remain double, unless it's opset <= 2.

ONNXRuntime does not yet support opset 3, so customer needs to specify onnx ml opset 2 for export. Is that achievable through target_opset argument in convert_sklearn? The description says number, for example, 7 for ONNX 1.2, and 8 for ONNX 1.3, which seems like onnx opset but not onnx ml opset.

@BowenBao BowenBao closed this Feb 15, 2022
@BowenBao BowenBao reopened this Feb 15, 2022
@xadupre
Copy link
Collaborator

xadupre commented Feb 16, 2022

The build is fixed in PR #827 (an example in the documentation refers to an onnx model on github and its location has changed).

Signed-off-by: BowenBao <bowbao@microsoft.com>
Signed-off-by: BowenBao <bowbao@microsoft.com>
Signed-off-by: BowenBao <bowbao@microsoft.com>
@BowenBao BowenBao force-pushed the bowbao/random_forest_zip_map_double branch from 2b65aaf to c2381a3 Compare February 22, 2022 22:46
@BowenBao BowenBao merged commit e9433de into onnx:master Feb 23, 2022
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.

2 participants