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

[Model Repository] Export as ONNX #190

Open
Brett-Parker opened this issue Jul 8, 2020 · 9 comments
Open

[Model Repository] Export as ONNX #190

Brett-Parker opened this issue Jul 8, 2020 · 9 comments
Assignees
Labels

Comments

@Brett-Parker
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
Roadmap

Describe the solution you'd like
Have the ability to store standard .zip model and ONNX model.

Additional context
Export/import as ONNX is possible with ML.NET. https://docs.microsoft.com/en-us/dotnet/api/microsoft.ml.onnxexportextensions.converttoonnx?view=ml-dotnet-preview

@Brett-Parker Brett-Parker added help wanted Extra attention is needed deployment labels Jul 8, 2020
@Brett-Parker
Copy link
Collaborator Author

@aslotte what did you have in mind for this?

@aslotte
Copy link
Owner

aslotte commented Jul 9, 2020

@Brett-Parker Thank you for creating this issue!

Jon did a great video on the topic actually https://www.youtube.com/watch?v=Yf7VXj1etdw and here's the official doc's https://docs.microsoft.com/en-us/dotnet/api/microsoft.ml.onnxexportextensions.converttoonnx?view=ml-dotnet-preview

We can solve this in many ways, e.g. always converting each uploaded model to ONNX and store for future use, or convert to ONNX when the user requests a model to be exported as ONNX. It will be a lot easier to do it for each uploaded model and store it, so that's what I want to go with :)

The user would need to also pass in the IDataView in the upload method.
The idea I have is to store the ONNX model next to the other model with the same runid (I think the ONNX model has another file format). Happy to discuss this further though as I've only given it about 10 min of thought.

@willvelida
Copy link
Collaborator

@aslotte I'm interested in picking this issue up 👍

I'm leaning towards letting the user request the model being exported to ONNX over converting each model to ONNX, but I'm interested to hear if you've had time to think further on this issue and hear what your thoughts are before diving in 😊

@aslotte
Copy link
Owner

aslotte commented Aug 14, 2020

We would welcome you to the team of contributors with open arms @willvelida! 😄

I agree, the more I've thought about it, the more I think we shouldn't convert each model to ONNX upon upload, especially since not all transformers are able to do so yet.

I would love to hear your thoughts on this. One option we can do is to make UploadModelAsync a lot less smart than what it is today and as such be more useful.

We can:

  • Set the RunArtifact name to the actual file name instead of {runId}.zip
  • Add a property to RunArtifact to store the file extension, e.g. .zip or .onnx
  • Modify the model repository methods to use the file endings of the RunArtifact when downloading a model or deploying it

There may be more details than that that need to be changed, but as such a user can decide themselves if the want to upload the ML.NET model or just the ONNX model directly (or both). I think that would make for a more re-usable approach and as such we can then in the future use RunArtifacts to store other things, e.g. if the user wants to upload data or notes as part of the run (via another method, e.g. LogRunArtifact).

(Side note: we still need the uploaded model to have the name of {runId}.{fileExtension} as it needs to be unique per run. The deployed model can have the name of {experimentName}.{fileExtension} as we only have one at a given time).

@willvelida
Copy link
Collaborator

I like adding a property to RunArtifact to store the file extension 😊 It seems a much more simpler approach than writing numerous overloads for different types of models, while keeping the integrity of the {runId}.{fileExtension} format for each run.

@aslotte
Copy link
Owner

aslotte commented Aug 15, 2020

@willvelida do you want me to assign this issue to do?

@willvelida
Copy link
Collaborator

@aslotte sure! 😊

@aslotte
Copy link
Owner

aslotte commented Aug 15, 2020

Awesome! I assigned the issue to you and also sent you a collaborator invitation for the repo so you can assign issues yourself in the future as well as review and merged reviewed PR's 😄

@aslotte
Copy link
Owner

aslotte commented Oct 3, 2020

@willvelida I added the hacktoberfest label to this issue in case someone would like to have a go at it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants