Open addisonklinke opened 3 years ago
Thanks for the issue @addisonklinke ! want to try and submit a PR?
I've added a draft PR trying to solve this issue. I've tackled it in a very inefficient way, but if this is the way to go, I'll be happy to clean it up and convert it into an actual PR.
@vballoli Thanks for getting the ball rolling on this one. I haven't had much time to work on a PR, but I will review yours and leave my comments there where applicable
This issue has been automatically marked as stale because it hasn't had any recent activity. This issue will be closed in 7 days if no further activity occurs. Thank you for your contributions, Pytorch Lightning Team!
PR (https://github.com/PyTorchLightning/pytorch-lightning/pull/7458) still in progress
🚀 Feature
The official PyTorch tutorial for exporting to ONNX includes checking the model spec as well as confirming the output values match the PyTorch version of the model. The current Lightning implementation of
to_onnx()
only callstorch.onnx.export
but skips these additional checks. For best practice, I think they should be included (if not by default, then with an optional boolean flag to turn them on)Motivation
Pitch
Add something along the lines of the following code to the end of the current
to_onnx()
methodAlternatives
Users override the method and implement the checks themselves. However, I think these are general enough that they should be included in the base method to limit boilerplate code
Additional context
onnx
andonnxruntime
with the Lightning distributionzip
is probably alright unless the PyTorch version outputs a dict (which I think would be bad practice)