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

CAMP-TUM network contributions: Quicknat and Daf3D #6306

Merged
merged 84 commits into from
Jun 24, 2023

Conversation

vgonzalezd
Copy link
Contributor

Fixes #5921

Description

@Al3xand1a and @ge96lip implement DAF3D[1] and Quicknat[2] networks and test them in open-source and local datasets. We use as a baseline the pytorch codes available in [3] and [4]
We are quite confident about the implementation, but feel free to contact us if you find errors. For any questions send us an email to ge45qix@mytum.de

We have some questions for the contribution:
For Quicknat:

  1. we add the sequential class file in the networks folder because we do not know where to add it.
  2. How do we include the squeeze and excitation requirement (sse and Csse) if it comes from a GitHub repository?
    https://github.com/ai-med/nn-common-modules/releases/download/v1.1/nn_common_modules-1.3-py3-none-any.whl
    For Daf3D
  3. Are the overwritten blocks fine as they are? or do they have to be more flexible?
    For both:
    We run this command line (./runtests.sh --quick --unittests --disttests) but the error we are getting is not related to our changes, so we run our unit_test independently and they work. Same with documentation.

[1] Deep Attentive Features for Prostate Segmentation in 3D Transrectal Ultrasound.
Yi Wang, Haoran Dou, Xiaowei Hu, Lei Zhu, Xin Yang, Ming Xu, Jing Qin, Pheng-Ann Heng, Tianfu Wang, and Dong Ni.
IEEE Transactions on Medical Imaging, 2019.
[2] Roy, A. G., Conjeti, S., Navab, N., Wachinger, C., & Alzheimer's Disease Neuroimaging Initiative. (2019). QuickNAT: A fully convolutional network for quick and accurate segmentation of neuroanatomy. NeuroImage, 186, 713-727.
[3] https://github.com/ai-med/quickNAT_pytorch
[4] https://github.com/wulalago/DAF3D

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.

Alexandra Marquardt and others added 28 commits February 5, 2023 12:35
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
@wyli wyli requested review from ericspod and Nic-Ma April 5, 2023 20:04
@wyli
Copy link
Member

wyli commented Jun 22, 2023

/black
It seems all the code reviews are addressed, thank you! there are two remaining test errors:

python -m pip install -U pip
python -m pip install -U -r requirements-dev.txt
./runtests.sh --mypy

please feel free to use # type: ignore if you think some lines are false alarms.

======================================================================
ERROR: test_script (tests.test_daf3d.TestDAF3D)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/MONAI/MONAI/tests/test_daf3d.py", line 56, in test_script
    test_script_save(net, test_data)
  File "/home/runner/work/MONAI/MONAI/tests/utils.py", line 731, in test_script_save
    convert_to_torchscript(
  File "/home/runner/work/MONAI/MONAI/monai/networks/utils.py", line 746, in convert_to_torchscript
    script_module = torch.jit.script(model, **kwargs)
  File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/torch/jit/_script.py", line 1286, in script
    return torch.jit._recursive.create_script_module(
  File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/torch/jit/_recursive.py", line 476, in create_script_module
    return create_script_module_impl(nn_module, concrete_type, stubs_fn)
  File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/torch/jit/_recursive.py", line 542, in create_script_module_impl
    create_methods_and_properties_from_stubs(concrete_type, method_stubs, property_stubs)
  File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/torch/jit/_recursive.py", line 393, in create_methods_and_properties_from_stubs
    concrete_type._create_methods_and_properties(property_defs, property_rcbs, method_defs, method_rcbs, method_defaults)
RuntimeError: 
Unexpected starred expansion. File a bug report:
  File "/home/runner/work/MONAI/MONAI/monai/networks/nets/daf3d.py", line 547
    
        attentive_features_maps = [self.attention(slf, mlf) for slf in single_layer_features]
        att_features, att_maps = zip(*attentive_features_maps)
                                     ~~~~~~~~~~~~~~~~~~~~~~~~ <--- HERE
    
        # second 4 supervised signals (af 1 - 4)


----------------------------------------------------------------------

I think the PR is ready to merge when these are fixed, please let us know if help needed cc @ericspod @Nic-Ma

… 126

Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Alexandra Marquardt and others added 7 commits June 22, 2023 14:40
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Signed-off-by: Al3xand1a <98582325+Al3xand1a@users.noreply.github.com>
Signed-off-by: Alexandra Marquardt <alexandra@MacBook-Pro-von-Alexandra.local>
@Al3xand1a
Copy link
Contributor

It seems all the code reviews are addressed, thank you! there are two remaining test errors:

python -m pip install -U pip
python -m pip install -U -r requirements-dev.txt
./runtests.sh --mypy

please feel free to use # type: ignore if you think some lines are false alarms.

======================================================================
ERROR: test_script (tests.test_daf3d.TestDAF3D)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/MONAI/MONAI/tests/test_daf3d.py", line 56, in test_script
    test_script_save(net, test_data)
  File "/home/runner/work/MONAI/MONAI/tests/utils.py", line 731, in test_script_save
    convert_to_torchscript(
  File "/home/runner/work/MONAI/MONAI/monai/networks/utils.py", line 746, in convert_to_torchscript
    script_module = torch.jit.script(model, **kwargs)
  File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/torch/jit/_script.py", line 1286, in script
    return torch.jit._recursive.create_script_module(
  File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/torch/jit/_recursive.py", line 476, in create_script_module
    return create_script_module_impl(nn_module, concrete_type, stubs_fn)
  File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/torch/jit/_recursive.py", line 542, in create_script_module_impl
    create_methods_and_properties_from_stubs(concrete_type, method_stubs, property_stubs)
  File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/torch/jit/_recursive.py", line 393, in create_methods_and_properties_from_stubs
    concrete_type._create_methods_and_properties(property_defs, property_rcbs, method_defs, method_rcbs, method_defaults)
RuntimeError: 
Unexpected starred expansion. File a bug report:
  File "/home/runner/work/MONAI/MONAI/monai/networks/nets/daf3d.py", line 547
    
        attentive_features_maps = [self.attention(slf, mlf) for slf in single_layer_features]
        att_features, att_maps = zip(*attentive_features_maps)
                                     ~~~~~~~~~~~~~~~~~~~~~~~~ <--- HERE
    
        # second 4 supervised signals (af 1 - 4)


----------------------------------------------------------------------

I think the PR is ready to merge when these are fixed, please let us know if help needed cc @ericspod @Nic-Ma

I fear I don't know what the torchscript bug is about. Why can't I use list expansion here? I appreciate any help!

ge96lip and others added 2 commits June 24, 2023 10:17
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
monai/networks/nets/daf3d.py Outdated Show resolved Hide resolved
Al3xand1a and others added 3 commits June 24, 2023 12:11
Co-authored-by: Wenqi Li <831580+wyli@users.noreply.github.com>
Signed-off-by: Al3xand1a <98582325+Al3xand1a@users.noreply.github.com>
Signed-off-by: Al3xand1a <98582325+Al3xand1a@users.noreply.github.com>
Signed-off-by: Wenqi Li <831580+wyli@users.noreply.github.com>
monai/networks/nets/daf3d.py Outdated Show resolved Hide resolved
Al3xand1a and others added 3 commits June 24, 2023 15:01
Signed-off-by: Al3xand1a <98582325+Al3xand1a@users.noreply.github.com>
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
@wyli
Copy link
Member

wyli commented Jun 24, 2023

/build

Copy link
Member

@wyli wyli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you! merging this if the test pipelines work fine

@wyli wyli enabled auto-merge (squash) June 24, 2023 14:02
@wyli wyli merged commit 0ffe55a into Project-MONAI:dev Jun 24, 2023
30 of 35 checks passed
drbeh pushed a commit to drbeh/MONAI that referenced this pull request Jun 26, 2023
Fixes Project-MONAI#5921 

### Description

@Al3xand1a and @ge96lip implement DAF3D[1] and Quicknat[2] networks and
test them in open-source and local datasets. We use as a baseline the
pytorch codes available in [3] and [4]
We are quite confident about the implementation, but feel free to
contact us if you find errors. For any questions send us an email to
ge45qix@mytum.de

We have some questions for the contribution:
For Quicknat:
1) we add the sequential class file in the networks folder because we do
not know where to add it.
2) How do we include the squeeze and excitation requirement (sse and
Csse) if it comes from a GitHub repository?

https://github.com/ai-med/nn-common-modules/releases/download/v1.1/nn_common_modules-1.3-py3-none-any.whl
For Daf3D
3) Are the overwritten blocks fine as they are? or do they have to be
more flexible?
For both:
We run this command line (`./runtests.sh --quick --unittests
--disttests`) but the error we are getting is not related to our
changes, so we run our unit_test independently and they work. Same with
documentation.
 
[1] Deep Attentive Features for Prostate Segmentation in 3D Transrectal
Ultrasound.
Yi Wang, Haoran Dou, Xiaowei Hu, Lei Zhu, Xin Yang, Ming Xu, Jing Qin,
Pheng-Ann Heng, Tianfu Wang, and Dong Ni.
IEEE Transactions on Medical Imaging, 2019.
[2] Roy, A. G., Conjeti, S., Navab, N., Wachinger, C., & Alzheimer's
Disease Neuroimaging Initiative. (2019). QuickNAT: A fully convolutional
network for quick and accurate segmentation of neuroanatomy. NeuroImage,
186, 713-727.
[3] https://github.com/ai-med/quickNAT_pytorch
[4] https://github.com/wulalago/DAF3D

### 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).
- [x] New tests added to cover the changes.
- [x] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [x] In-line docstrings updated.
- [x] Documentation updated, tested `make html` command in the `docs/`
folder.

---------

Signed-off-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Signed-off-by: vanessagd.2395 <vanessa.gonzalez-duque@eleves.ec-nantes.fr>
Signed-off-by: Al3xand1a <98582325+Al3xand1a@users.noreply.github.com>
Co-authored-by: Alexandra Marquardt <alexandra@MBPvonAlexandra.fritz.box>
Co-authored-by: Carlotta <carlotta.hoelzle@icloud.com>
Co-authored-by: Alexandra Marquardt <alexandra@w223-2e-v4.eduroam.dynamic.rbg.tum.de>
Co-authored-by: Vanessa <vanessa.gonzalezduque@ls2n.fr>
Co-authored-by: ge96lip <73938628+ge96lip@users.noreply.github.com>
Co-authored-by: “Vanessa <“vanessa.gonzalezduque@ls2n.fr”>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Alexandra Marquardt <alexandra@w192-1i-v4.eduroam.dynamic.rbg.tum.de>
Co-authored-by: Al3xand1a <98582325+Al3xand1a@users.noreply.github.com>
Co-authored-by: Alexandra Marquardt <alexandra@w217-4n-v4.eduroam.dynamic.rbg.tum.de>
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.

TUM-CAMP-Networks-Contribution
5 participants