Skip to content

SaveOnnxCommand appears to ignore predictors when saving a model to ONNX format. #3974

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
pieths opened this issue Jul 8, 2019 · 1 comment · Fixed by #3986
Closed

SaveOnnxCommand appears to ignore predictors when saving a model to ONNX format. #3974

pieths opened this issue Jul 8, 2019 · 1 comment · Fixed by #3986
Assignees
Labels
P0 Priority of the issue for triage purpose: IMPORTANT, needs to be fixed right away.

Comments

@pieths
Copy link
Contributor

pieths commented Jul 8, 2019

System information

  • OS version/distro: Windows 10

Steps To Recreate The Issue

  1. Create and save a PredictorModel to disk using the entry point api.

  2. Try and convert the model to ONNX format using the entry point api.

  3. Notice that SaveOnnxCommand.GetPipe only cycles through the transforms and never encounters the logistic regression node.

    This might be happening because ExecuteGraphCommand.GetOutputToPath saves a TlcModule.DataKind.PredictorModel to disk in step (1). And then, ExecuteGraphCommand.SetInputFromPath loads a TlcModule.DataKind.TransformModel from disk in step (2) (apparently a consequence of SaveOnnxCommand.Arguments.Model being of type TransformModel). PredictorModelImpl and TransformModelImpl don't appear to be compatible from a serialization point of view.

Source code / logs

See here for an ml.net test which demonstrates the issue.

@wschin
Copy link
Member

wschin commented Jul 10, 2019

Thanks a lot for pointing this out. I just made a solution based on your finding.

@codemzs codemzs added the P0 Priority of the issue for triage purpose: IMPORTANT, needs to be fixed right away. label Jul 12, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P0 Priority of the issue for triage purpose: IMPORTANT, needs to be fixed right away.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants