Lightning-AI / Lightning-AI/pytorch-lightning

Add checks for model spec and matching output values in `to_onnx()` method

Open
#7,279 6 comments 2 reactions 2 assignees View on GitHub

@addisonklinke is already working on this.

Since Jul 1, 2021.

feature good first issue help wanted
Dominant language
Python
Stars
31.4k
Forks
3.8k
Avg merge
6d 7h
Merged PRs (30d)
6

Description

## 🚀 Feature
The official PyTorch [tutorial](https://pytorch.org/tutorials/advanced/super_resolution_with_onnxruntime.html) 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 calls `torch.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

1. Follow the official tutorial as closely as possible
2. Ensure the ONNX version of the model does not produce unexpected results before it's used in production

### Pitch

Add something along the lines of the following code to the end of the current `to_onnx()` method

```python
import onnx
import onnxruntime
import numpy as np

def to_numpy(tensor):
return tensor.detach().cpu().numpy() if tensor.requires_grad else tensor.cpu().numpy()

onnx_model = onnx.load(file_path)
onnx.checker.check_model(onnx_model)
ort_session = onnxruntime.InferenceSession(file_path)
ort_inputs = {ort_session.get_inputs()[0].name: to_numpy(input_sample)}
ort_outs = ort_session.run(None, ort_inputs)
for ort_out, torch_out in zip(kwargs["example_outputs"], ort_outs):
np.testing.assert_allclose(to_numpy(torch_out), ort_out, rtol=1e-03, atol=1e-05)
```

### Alternatives

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

- This would require including the Python bindings for `onnx` and `onnxruntime` with the Lightning distribution
- The biggest challenge to generalize would be the model outputs. Many models have more than one output node, so we need to make sure each node is compared with the appropriate corresponding ONNX node. Iterating with `zip` is probably alright unless the PyTorch version outputs a dict (which I think would be bad practice)
- In my own use, I have found you need to be very careful about the order of library imports. Details in this issue [comment](https://github.com/onnx/onnx/issues/2394#issuecomment-581638840) from the onnx repo

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.