-
Notifications
You must be signed in to change notification settings - Fork 178
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
ONNX export support for LibLinear and LibSVM #191
Conversation
…asses at the moment due to a label permutation issue.
…babilistic models still don't work.
… the ensemble test.
} | ||
|
||
/** | ||
* Rationalises the output of an onnx model into a standard format suitable for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably mention the input types that it expects
Interop/ONNX/src/main/java/org/tribuo/interop/onnx/LabelOneVOneTransformer.java
Outdated
Show resolved
Hide resolved
OnnxTensor outputScores = (OnnxTensor) inputs.get(0); | ||
if (outputScores.getInfo().type == OnnxJavaType.FLOAT) { | ||
long[] shape = outputScores.getInfo().getShape(); | ||
if ((shape.length == 2) && (shape[1] == (outputIDInfo.size() * ((long) outputIDInfo.size() - 1) / 2))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this have the || shape[1] == 2
as the below clause does?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. ONNX Runtime automatically unfolds the output of a SVMClassifier so it has two columns, but in general if you're building (n * n-1) / 2
classifiers for your n
class problem you'll end up with 1 output for binary problems, and if ONNX Runtime hasn't special cased it you'll produce a single output tensor of size [batch_size,1]
which the other logic covers. Neither ONNX Runtime or the ONNX spec are really correct here, so I added some docs to explain that we're in an area prone to change because it's implementation dependent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good apart from minor changes in comments
…eTransformer.java Co-authored-by: Jack Sullivan <john.t.sullivan@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Description
Adds ONNX export support for LibLinear and LibSVM classification and regression models. It doesn't add anomaly detection export support, we'll leave that till a future release.
The LibSVM classification export depends on the exact behaviour of ONNX Runtime as the ONNX spec for SVMClassifier is incorrect - microsoft/onnxruntime#9429.
Motivation
We'd like to export models to ONNX. Also LibSVM uses an operator from the ONNX-ML set and so is a good test of the support.