Skip to content

6219 fixes type annotations for tensorrt #6229

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

Merged
merged 7 commits into from
Mar 24, 2023
Merged

Conversation

wyli
Copy link
Contributor

@wyli wyli commented Mar 23, 2023

fixes #6219

Description

this makes monai.networks type annotations unchanged

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).
  • 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.

wyli added 2 commits March 23, 2023 11:19
Signed-off-by: Wenqi Li <[email protected]>
Signed-off-by: Wenqi Li <[email protected]>
@wyli wyli marked this pull request as ready for review March 23, 2023 11:34
@wyli wyli requested review from binliunls and Nic-Ma March 23, 2023 11:34
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Mar 23, 2023

Hi @binliunls ,

Could you please help verify this PR with PyTorch 23.03 docker?
BTW, I think you can also test all the TensorRT supported bundles, if some network also breaks with typehint issue, let
's update in this PR.

Thanks in advance.

@wyli wyli changed the title test type hint fixes type annotations for tensorrt Mar 23, 2023
@wyli wyli changed the title fixes type annotations for tensorrt 6219 fixes type annotations for tensorrt Mar 23, 2023
@binliunls
Copy link
Contributor

binliunls commented Mar 24, 2023

Hi @wyli and @Nic-Ma ,
When I run the code below to convert dynunet to torchscript in MONAI docker 1.2.0rc2, I got some error. This error wouldn't be solved until I replaced all the | type annotation with typing format Optional and removed the from __future__ import annotations line. Please notice the error still existed when I just replace all the | with Optional but leave the from __future__ import annotations at the top of the code.
@wyli, I am not an expert in torchscript, but I think this kind of error should be relate to the torchscript lacks support for the new annotation type. I can update this one as I did in the debug process. But some other networks not tested may also have the same issue.

import torch
from monai.networks.nets import DynUNet

if __name__ == "__main__":
   model = DynUNet(
       spatial_dims=3,
       in_channels=3,
       out_channels=2,
       kernel_size=[3, 3, 3, 3, 3, 3],
       strides=[1, 2, 2, 2, 2, [2, 2, 1]],
       upsample_kernel_size=[2, 2, 2, 2, [2, 2, 1]],
       norm_name="instance",
       deep_supervision=False,
       res_block=True,
   )
   model.eval()
   with torch.no_grad():
       script_model = torch.jit.script(model)

Error output:

Traceback (most recent call last):
  File "/usr/lib/python3.8/runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.8/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/root/.vscode-server/extensions/ms-python.python-2023.4.1/pythonFiles/lib/python/debugpy/adapter/../../debugpy/launcher/../../debugpy/__main__.py", line 39, in <module>
    cli.main()
  File "/root/.vscode-server/extensions/ms-python.python-2023.4.1/pythonFiles/lib/python/debugpy/adapter/../../debugpy/launcher/../../debugpy/../debugpy/server/cli.py", line 430, in main
    run()
  File "/root/.vscode-server/extensions/ms-python.python-2023.4.1/pythonFiles/lib/python/debugpy/adapter/../../debugpy/launcher/../../debugpy/../debugpy/server/cli.py", line 284, in run_file
    runpy.run_path(target, run_name="__main__")
  File "/root/.vscode-server/extensions/ms-python.python-2023.4.1/pythonFiles/lib/python/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_runpy.py", line 321, in run_path
    return _run_module_code(code, init_globals, run_name,
  File "/root/.vscode-server/extensions/ms-python.python-2023.4.1/pythonFiles/lib/python/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_runpy.py", line 135, in _run_module_code
    _run_code(code, mod_globals, init_globals,
  File "/root/.vscode-server/extensions/ms-python.python-2023.4.1/pythonFiles/lib/python/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_runpy.py", line 124, in _run_code
    exec(code, run_globals)
  File "/home/liubin/data/tmp/test.py", line 18, in <module>
    script_model = torch.jit.script(model)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_script.py", line 1286, in script
    return torch.jit._recursive.create_script_module(
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 476, in create_script_module
    concrete_type = get_module_concrete_type(nn_module, share_types)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 427, in get_module_concrete_type
    concrete_type = concrete_type_store.get_or_create_concrete_type(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 368, in get_or_create_concrete_type
    concrete_type_builder = infer_concrete_type_builder(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 233, in infer_concrete_type_builder
    sub_concrete_type = get_module_concrete_type(item, share_types)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 427, in get_module_concrete_type
    concrete_type = concrete_type_store.get_or_create_concrete_type(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 368, in get_or_create_concrete_type
    concrete_type_builder = infer_concrete_type_builder(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 233, in infer_concrete_type_builder
    sub_concrete_type = get_module_concrete_type(item, share_types)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 427, in get_module_concrete_type
    concrete_type = concrete_type_store.get_or_create_concrete_type(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 368, in get_or_create_concrete_type
    concrete_type_builder = infer_concrete_type_builder(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 233, in infer_concrete_type_builder
    sub_concrete_type = get_module_concrete_type(item, share_types)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 427, in get_module_concrete_type
    concrete_type = concrete_type_store.get_or_create_concrete_type(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 368, in get_or_create_concrete_type
    concrete_type_builder = infer_concrete_type_builder(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 233, in infer_concrete_type_builder
    sub_concrete_type = get_module_concrete_type(item, share_types)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 427, in get_module_concrete_type
    concrete_type = concrete_type_store.get_or_create_concrete_type(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 368, in get_or_create_concrete_type
    concrete_type_builder = infer_concrete_type_builder(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 233, in infer_concrete_type_builder
    sub_concrete_type = get_module_concrete_type(item, share_types)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 427, in get_module_concrete_type
    concrete_type = concrete_type_store.get_or_create_concrete_type(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 368, in get_or_create_concrete_type
    concrete_type_builder = infer_concrete_type_builder(nn_module)
  File "/usr/local/lib/python3.8/dist-packages/torch/jit/_recursive.py", line 243, in infer_concrete_type_builder
    if torch._jit_internal.is_final(ann):
  File "/usr/local/lib/python3.8/dist-packages/torch/_jit_internal.py", line 1064, in is_final
    return ann.__module__ in {"typing", "typing_extensions"} and (
AttributeError: 'str' object has no attribute '__module__'

Thanks,
Bin

@binliunls
Copy link
Contributor

binliunls commented Mar 24, 2023

Hi @binliunls ,

Could you please help verify this PR with PyTorch 23.03 docker? BTW, I think you can also test all the TensorRT supported bundles, if some network also breaks with typehint issue, let 's update in this PR.

Thanks in advance.

Hi @Nic-Ma ,
I have tested the 7 successfully converted models, here are the results.
The spleen_ct_segmentation, endoscopic_tool_segmentation, endoscopit_inbody_classification and pathology_tumor_detection can be exported successfully in the current pytorch 23.03 docker, while endoscopic_inbody_classification still suffers the slowdown from directly torch-trt exporting.
The torch_tensorrt PRs fixed the brats_mri_segmentation and lung_nodule_ct_detection bundles don't seem to be merged in the currently 23.03 docker which lead to a failure export. We can only convert these bundle by recompiling the main branch of Torch-TensorRT in the docker. Not sure if they will be merged in the public release version of 23.03.
The PR fixed spleen_deepedit_annotation bundle hasn't been merged to the main branch and the new dynunet also has torchscript convert issue. With recompiled torch-tensorrt with PR and the type hint | to Optional type change, this bundle can be converted successfully.

Thanks,
Bin

@binliunls
Copy link
Contributor

binliunls commented Mar 24, 2023

And the dints also has the torchscript convert issue. Here is the error output:

 Expression of type | cannot be used in a type expression:
File "/home/liubin/data/github_monai/6219-type-hint/MONAI/monai/networks/nets/dints.py", line 302
def forward(self, x: torch.Tensor, weight: torch.Tensor | None) -> torch.Tensor:
	                                 ~~~~~~~~~~~~~~~~~~~ <--- HERE
      """
      Args:

@wyli
Copy link
Contributor Author

wyli commented Mar 24, 2023

sure, do you plan to add more code changes @binliunls?

@binliunls
Copy link
Contributor

sure, do you plan to add more code changes @binliunls?

Hi, @wyli
I have tested all previously tested bundles and @yiheng-wang-nv also helped on test models in model zoo. I think currently there is no more code change plan for this PR from my side.

Thanks,
Bin

@wyli
Copy link
Contributor Author

wyli commented Mar 24, 2023

/build

@wyli
Copy link
Contributor Author

wyli commented Mar 24, 2023

sure, thanks @binliunls, when you approve this, I'll merge it into dev

@binliunls
Copy link
Contributor

sure, thanks @binliunls, when you approve this, I'll merge it into dev

@wyli One more thing is that should we make another release candidate later after pytorch 23.03 is avaliable? Otherwise there may be some issues for the model-zoo's CI/CD test. It's just a suggestion, since there may be some other works you'd like to add to the next release candidate version.

Thanks,
Bin

@wyli
Copy link
Contributor Author

wyli commented Mar 24, 2023

@binliunls sure there are a few integration issues, I'll address those first and let you know when a new rc is tagged. (it'll still be based on 23.02 though, as 23.03 is not publicly available yet)

@wyli wyli enabled auto-merge (squash) March 24, 2023 08:41
@wyli wyli merged commit c9889ce into Project-MONAI:dev Mar 24, 2023
@wyli wyli deleted the 6219-type-hint branch March 25, 2023 08:44
jak0bw pushed a commit to jak0bw/MONAI that referenced this pull request Mar 28, 2023
fixes Project-MONAI#6219 

### Description
this makes monai.networks type annotations unchanged


### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [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: Wenqi Li <[email protected]>
Signed-off-by: binliu <[email protected]>
Co-authored-by: binliu <[email protected]>
jak0bw pushed a commit to jak0bw/MONAI that referenced this pull request Mar 28, 2023
fixes Project-MONAI#6219 

### Description
this makes monai.networks type annotations unchanged


### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [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: Wenqi Li <[email protected]>
Signed-off-by: binliu <[email protected]>
Co-authored-by: binliu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot convert endoscopic_tool_segmentation model to TensorRT
3 participants