Add onnx export with verification by liqunfu · Pull Request #6237 · Project-MONAI/MONAI

liqunfu · 2023-03-26T00:20:55Z

Fixes # Issue#TBD.

Description

A MONAI Bundle can contain models in PyTorch, TorchScript, and ONNX formats.
Utility exists in MONAI to export a model in TorchScript format. Exported TorchScript models can be verified with Torch runtime.
It is helpful (and complete) to provide a same utility to export ONNX models and to validate the models with PyTorch, ONNX(https://github.com/onnx/onnx), and onnxruntime (https://github.com/microsoft/onnxruntime).

With the proposed utility, ONNX model can be added to bundles in model-zoo with calling monai.bundle ckpt_export.

Types of changes

Non-breaking change (fix or new feature that would not break existing functionality).
Breaking change (fix or new feature that would cause existing functionality to change).
[x ] New tests added to cover the changes.
Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
In-line docstrings updated.
Documentation updated, tested make html command in the docs/ folder.

./runtests.sh --quick --unittests --disttests
TOTAL 72191 11290 84%

Signed-off-by: Liqun Fu <[email protected]>

monai/networks/utils.py

tests/test_convert_to_onnx.py

monai/bundle/scripts.py

binliunls · 2023-03-29T08:56:12Z

Hi @liqunfu ,
Do you have an ETA for this PR? We are working on this too. If you can have this PR merged this week, we can wait. Otherwise we can start a PR for this and invite you to review it.

Thanks,
Bin

liqunfu · 2023-03-29T15:59:59Z

Hi @liqunfu , Do you have an ETA for this PR? We are working on this too. If you can have this PR merged this week, we can wait. Otherwise we can start a PR for this and invite you to review it.

Thanks, Bin

I am starting today to update my PR with reviewers' comments and to fix CI failures. I was trying to add more test for models that are used by model-zoo but it is time consuming because I need to get evaluation data some of which also need to wait for access approval. I agree with @ericspod that it can be in another PR.

BTW thanks all for reviewing this PR. I will update it by the end of today.

liqunfu · 2023-03-29T17:19:46Z

Hi @liqunfu , Do you have an ETA for this PR? We are working on this too. If you can have this PR merged this week, we can wait. Otherwise we can start a PR for this and invite you to review it.
Thanks, Bin

I am starting today to update my PR with reviewers' comments and to fix CI failures. I was trying to add more test for models that are used by model-zoo but it is time consuming because I need to get evaluation data some of which also need to wait for access approval. I agree with @ericspod that it can be in another PR.

BTW thanks all for reviewing this PR. I will update it by the end of today.

@binliunls Please feel free to add to this PR with what you may already have so we can merge sooner.

Signed-off-by: Liqun Fu <[email protected]>

…st because it failed with torch < 1.12 Signed-off-by: Liqun Fu <[email protected]>

for more information, see https://pre-commit.ci

Signed-off-by: Liqun Fu <[email protected]>

…into liqun/export-onnx

Signed-off-by: Liqun Fu <[email protected]>

for more information, see https://pre-commit.ci

Signed-off-by: Liqun Fu <[email protected]>

.github/workflows/pythonapp.yml

Nic-Ma

Hi @liqunfu ,

About adding a new optional dependency package, please refer to:
https://github.com/Project-MONAI/MONAI/blob/dev/CONTRIBUTING.md#adding-new-optional-dependencies

Thanks.

monai/bundle/scripts.py

Nic-Ma · 2023-03-30T01:17:54Z

Mark: This PR will be targeting MONAI 1.2 P1 feature as several bundles which can't be supported by torch-trt so far can leverage ONNX path to convert to TensorRT model.

Thanks.

Signed-off-by: Liqun Fu <[email protected]>

…dency Signed-off-by: binliu <[email protected]>

Signed-off-by: binliu <[email protected]>

binliunls · 2023-04-01T03:35:49Z

Hi @wyli ,
The windows-latest ci/cd test reports OOM error. Could you please give me some advice on why this happened and how to fix it?

error:

======================================================================
ERROR: test_vnet_shape_5 (tests.test_vnet.TestVNet)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\site-packages\parameterized\parameterized.py", line 620, in standalone_func
    return func(*(a + p.args), **p.kwargs, **kw)
  File "D:\a\MONAI\MONAI\tests\test_vnet.py", line 69, in test_vnet_shape
    net = VNet(**input_param).to(device)
  File "D:\a\MONAI\MONAI\monai\networks\nets\vnet.py", line 241, in __init__
    self.up_tr256 = UpTransition(spatial_dims, 256, 256, 2, act, dropout_prob=dropout_prob)
  File "D:\a\MONAI\MONAI\monai\networks\nets\vnet.py", line 151, in __init__
    self.ops = _make_nconv(spatial_dims, out_channels, nconvs, act)
  File "D:\a\MONAI\MONAI\monai\networks\nets\vnet.py", line 55, in _make_nconv
    layers.append(LUConv(spatial_dims, nchan, act, bias))
  File "D:\a\MONAI\MONAI\monai\networks\nets\vnet.py", line 36, in __init__
    self.conv_block = Convolution(
  File "D:\a\MONAI\MONAI\monai\networks\blocks\convolutions.py", line 143, in __init__
    conv = conv_type(
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\site-packages\torch\nn\modules\conv.py", line 591, in __init__
    super(Conv3d, self).__init__(
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\site-packages\torch\nn\modules\conv.py", line 137, in __init__
    self.weight = Parameter(torch.empty(
RuntimeError: [enforce fail at C:\actions-runner\_work\pytorch\pytorch\builder\windows\pytorch\c10\core\impl\alloc_cpu.cpp:72] data. DefaultCPUAllocator: not enough memory: you tried to allocate 32768000 bytes.

======================================================================
FAIL: test_cachedataset (tests.test_sampler_dist.DistributedSamplerTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\MONAI\MONAI\tests\utils.py", line 539, in _wrapper
    assert results.get(), "Distributed call failed."
AssertionError: Distributed call failed.

Thanks,
Bin

Nic-Ma

Looks good to me, put some minor comments inline.
@wyli Do you think we need to add all the optional packages in this function or only selected?
https://github.com/Project-MONAI/MONAI/blob/dev/monai/config/deviceconfig.py#L64

Thanks.

monai/bundle/scripts.py

setup.cfg

monai/bundle/scripts.py

setup.cfg

Signed-off-by: binliu <[email protected]>

…into liqun/export-onnx

for more information, see https://pre-commit.ci

Signed-off-by: binliu <[email protected]>

Signed-off-by: binliunls <[email protected]>

Signed-off-by: binliu <[email protected]>

Nic-Ma

Thanks for the update, looks good to me.
Please fix the failed CI tests on windows, if it's expected error, please add skip_if_windows decorator.

Thanks.

monai/bundle/scripts.py

tests/test_bundle_onnx_export.py

Signed-off-by: binliu <[email protected]>

…ast time this PR pass windows CI Signed-off-by: Liqun Fu <[email protected]>

Signed-off-by: Liqun Fu <[email protected]>

wyli · 2023-04-04T21:38:40Z

/build

wyli · 2023-04-04T22:46:45Z

/build

wyli · 2023-04-05T07:20:43Z

/build

This reverts commit 9b09ed6.

Fixes #6258 . ### Description Add onnx as an option for converting pytorch models to TensorRT models through `trt_export` API. This pr depends on #6237 to add the onnx convert function. ### Types of changes  - [x] Non-breaking change (fix or new feature that would not break existing functionality). - [ ] Breaking change (fix or new feature that would cause existing functionality to change). - [ ] New tests added to cover the changes. - [ ] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [ ] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [ ] In-line docstrings updated. - [ ] Documentation updated, tested `make html` command in the `docs/` folder. --------- Signed-off-by: binliu <[email protected]>

add onnx export with verification

ec0ebcc

Signed-off-by: Liqun Fu <[email protected]>

liqunfu marked this pull request as draft March 26, 2023 00:21

liqunfu mentioned this pull request Mar 26, 2023

Add ONNX support (RE: PR #6237) #6238

Closed

wyli requested review from Nic-Ma, binliunls and ericspod March 27, 2023 10:46

Nic-Ma requested a review from yiheng-wang-nv March 27, 2023 14:36

ericspod reviewed Mar 27, 2023

View reviewed changes

monai/networks/utils.py Show resolved Hide resolved

ericspod reviewed Mar 27, 2023

View reviewed changes

tests/test_convert_to_onnx.py Show resolved Hide resolved

binliunls reviewed Mar 28, 2023

View reviewed changes

monai/bundle/scripts.py Outdated Show resolved Hide resolved

update with reviewers comments, try fix CI failures

63d9c8d

Signed-off-by: Liqun Fu <[email protected]>

liqunfu changed the title ~~WIP: add onnx export with verification~~ Add onnx export with verification Mar 29, 2023

liqunfu marked this pull request as ready for review March 29, 2023 19:49

liqunfu and others added 10 commits March 29, 2023 12:49

Merge branch 'dev' into liqun/export-onnx

8f5b8f7

MacOS not to use ort-cpu (not available on pypi), remove segResNet te…

3b3e819

…st because it failed with torch < 1.12 Signed-off-by: Liqun Fu <[email protected]>

[pre-commit.ci] auto fixes from pre-commit.com hooks

568ea3f

for more information, see https://pre-commit.ci

remove unused reference

3b185ed

Signed-off-by: Liqun Fu <[email protected]>

Merge branch 'liqun/export-onnx' of https://github.com/liqunfu/MONAI …

9946c2f

…into liqun/export-onnx

more on CI failures, bring back SegResNet test with pytorch condition

499f7ec

Signed-off-by: Liqun Fu <[email protected]>

remove ort-gpu because its incompatible with torch cuda

32590ac

Signed-off-by: Liqun Fu <[email protected]>

[pre-commit.ci] auto fixes from pre-commit.com hooks

54a8664

for more information, see https://pre-commit.ci

Merge branch 'dev' into liqun/export-onnx

eef309c

flake8 fix

b674c9f

Signed-off-by: Liqun Fu <[email protected]>

Nic-Ma reviewed Mar 30, 2023

View reviewed changes

.github/workflows/pythonapp.yml Outdated Show resolved Hide resolved

Nic-Ma reviewed Mar 30, 2023

View reviewed changes

monai/bundle/scripts.py Outdated Show resolved Hide resolved

update with reviewer's comment

d119378

Signed-off-by: Liqun Fu <[email protected]>

binliunls added 2 commits April 1, 2023 01:35

add test_convert_to_onnx to exclude list since it uses the onnx depen…

e421371

…dency Signed-off-by: binliu <[email protected]>

add onnx.reference optional import

1662cf6

Signed-off-by: binliu <[email protected]>

binliunls mentioned this pull request Apr 3, 2023

6258-add-onnx-option-to-trt-export #6274

Merged

7 tasks

Nic-Ma approved these changes Apr 3, 2023

View reviewed changes

monai/bundle/scripts.py Outdated Show resolved Hide resolved

monai/bundle/scripts.py Outdated Show resolved Hide resolved

setup.cfg Outdated Show resolved Hide resolved

Nic-Ma reviewed Apr 3, 2023

View reviewed changes

monai/bundle/scripts.py Outdated Show resolved Hide resolved

Nic-Ma reviewed Apr 3, 2023

View reviewed changes

setup.cfg Outdated Show resolved Hide resolved

binliunls and others added 8 commits April 3, 2023 20:22

Merge branch 'dev' into liqun/export-onnx

2e2c11e

update the onnx optional version, has_onnx and some doc-string

ff90748

Signed-off-by: binliu <[email protected]>

Merge branch 'liqun/export-onnx' of https://github.com/liqunfu/MONAI …

8e3cfd3

…into liqun/export-onnx

[pre-commit.ci] auto fixes from pre-commit.com hooks

a462c99

for more information, see https://pre-commit.ci

add the python max version condition to the setup file

61f7f39

Signed-off-by: binliu <[email protected]>

add the run_workflow to the bundle __main__.py

6862cc4

Signed-off-by: binliu <[email protected]>

Merge branch 'dev' into liqun/export-onnx

ead6717

Signed-off-by: binliunls <[email protected]>

update the atol of of retinanet unit onnx save test

e7d1a33

Signed-off-by: binliu <[email protected]>

Nic-Ma approved these changes Apr 4, 2023

View reviewed changes

monai/bundle/scripts.py Outdated Show resolved Hide resolved

tests/test_bundle_onnx_export.py Show resolved Hide resolved

binliunls and others added 5 commits April 4, 2023 07:53

add blank space in doc-string

a640df7

Signed-off-by: binliu <[email protected]>

disable TestRetinaNet.test_onnx on windows - it was added after the l…

bc64ec9

…ast time this PR pass windows CI Signed-off-by: Liqun Fu <[email protected]>

trying to fix configure-pagefile-action failure

3a80a98

Signed-off-by: Liqun Fu <[email protected]>

bring back onnx_export test in test_retinanet.py

f1334ff

Signed-off-by: Liqun Fu <[email protected]>

Merge branch 'dev' into liqun/export-onnx

cd7b8ef

wyli enabled auto-merge (squash) April 4, 2023 21:38

wyli merged commit 9b09ed6 into Project-MONAI:dev Apr 5, 2023

wyli added a commit that referenced this pull request Apr 5, 2023

Revert "Add onnx export with verification (#6237)"

ec543eb

This reverts commit 9b09ed6.

wyli mentioned this pull request Apr 5, 2023

Revert "Add onnx export with verification" #6297

Closed

liqunfu mentioned this pull request May 30, 2023

SlidingWindowInferer pre- and post-processing (PNP) operator onnx/onnx#5270

Open

Conversation

liqunfu commented Mar 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

binliunls commented Mar 29, 2023

Uh oh!

liqunfu commented Mar 29, 2023

Uh oh!

liqunfu commented Mar 29, 2023

Uh oh!

Uh oh!

Nic-Ma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Nic-Ma commented Mar 30, 2023

Uh oh!

binliunls commented Apr 1, 2023

Uh oh!

Nic-Ma left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nic-Ma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wyli commented Apr 4, 2023

Uh oh!

wyli commented Apr 4, 2023

Uh oh!

wyli commented Apr 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

liqunfu commented Mar 26, 2023 •

edited

Loading

Nic-Ma left a comment •

edited

Loading