Skip to content

ONNX outputs don't match model outputs after serialization #2980

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

Closed
rogancarr opened this issue Mar 15, 2019 · 5 comments · Fixed by #4849
Closed

ONNX outputs don't match model outputs after serialization #2980

rogancarr opened this issue Mar 15, 2019 · 5 comments · Fixed by #4849
Assignees
Labels
bug Something isn't working P3 Doc bugs, questions, minor issues, etc. usability Smoothing user interaction or experience

Comments

@rogancarr
Copy link
Contributor

When I save a model to ONNX, load, and apply it with ApplyOnnxModel, it adds a zero as a suffix to all columns, including the expected output. This includes input columns, so the resulting IDataView now has double the columns, plus the output. To top it off, it's not clear how to find the output because it's been renamed.

Example:
ML.NET model
Input: Features
Output: Score
Resulting Schema: Features, Score

After Onnx Serialization / Deserialization:
Input: Features
Output: Features0, Score0
Resulting Schema: Features, Features0, Score0

I am not sure if this is by design, but it feels like a bug.

@rogancarr rogancarr added bug Something isn't working usability Smoothing user interaction or experience labels Mar 15, 2019
@jignparm
Copy link
Contributor

What does the model above produce as outputs (try this viewer https://github.com/lutzroeder/netron/releases/tag/v2.9.4) ? The zeros at the end of the names seems odd -- unless those are the names of the model outputs.

The OnnxTransform will generate output all output columns by default, with names, type and shape specified by the model, unless the user specifies explicitly the list of columns they are interested in.

@rogancarr
Copy link
Contributor Author

@jignparm Check out this PR: #2984 . I think the issue is the way we save or load the model.

@jignparm
Copy link
Contributor

@rogancarr , looks like the PR is merged.

For issues #2980, #2981 and #2982, the likely reason for the behaviour is the way the model is generated, as you mentioned above, using the function mlContext.Model.ConvertToOnnx(model, data, file);.

The OnnxTransform should honor the type, shape and names as specified by the model. In case there's a discrepancy with the OnnxTransform, that can be tracked in a separate issue.

@rogancarr
Copy link
Contributor Author

@jignparm The PR for tests is merged, but I'm still investigating this particular behavior. I'll post here once I've updated. I do think that it's in the function you call out.

@rogancarr rogancarr self-assigned this Apr 1, 2019
@singlis singlis self-assigned this Apr 1, 2019
@rogancarr rogancarr assigned singlis and unassigned singlis and rogancarr Apr 4, 2019
@singlis
Copy link
Member

singlis commented Apr 5, 2019

The "0" is because we are preserving the input column to imitate the behavior of what we would have in ml.net where the original columns are preserved as part of the output. In order to preserve the input, we are passing the input through an identity transform. ONNX can't have multiple inputs/outputs of the same name, so ML.NET generates the new output name by appending a 0 to the name:
image

This is by design. If we want true "end-to-end" symmetry with an imported ONNX model in that we have the same outputs as the original ML.NET model, then we would have to add CopyColumn transforms that rename the column back to the original column.

I feel like this is treading on the doing things automagically so I will add @TomFinley in case he has some thoughts, but from the stance of having an explicit API, then we don't want to do a conversion and this behavior is OK. If the user wants to rename back to the original, they can add a CopyColumn to rename.

@frank-dong-ms-zz frank-dong-ms-zz added the P3 Doc bugs, questions, minor issues, etc. label Jan 9, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working P3 Doc bugs, questions, minor issues, etc. usability Smoothing user interaction or experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants