Skip to content

Adding EfficientNetsB0-B7 support#1938

Merged
wyli merged 19 commits intoProject-MONAI:masterfrom
masadcv:1090-efficientnet-support
Apr 9, 2021
Merged

Adding EfficientNetsB0-B7 support#1938
wyli merged 19 commits intoProject-MONAI:masterfrom
masadcv:1090-efficientnet-support

Conversation

@masadcv
Copy link
Copy Markdown
Contributor

@masadcv masadcv commented Apr 3, 2021 •
edited
Loading

Signed-off-by: masadcv [email protected]

Fixes #1090 .

Description

Adding support for original EfficientNetB0-B7 models from: https://arxiv.org/abs/1905.11946

Based on https://github.com/lukemelas/EfficientNet-PyTorch

  • Extended to use MONAI factories to generalise for spatial_dims in [1, 2, 3]
  • Can be used with pretrained weights from above repo when spatial_dims=2
  • Also adds a memory-efficient version of Swish activation function from: Memory Issues lukemelas/EfficientNet-PyTorch#18 (comment) which is reported to save 30% memory during training on NVIDIA GPUs
  • Added unittests to cover checking output shapes for EfficientnetB0-B7 implementation variations and correct loading of pretrained weights from above repo
  • Added unittests to cover saving/loading with Torchscript using tests from tests.utils.test_script_save()
  • Added a kitty test image for classification models unittesting at: ./tests/testing_data/kitty_test.jpg (image from: https://commons.wikimedia.org/wiki/File:Tabby_cat_with_blue_eyes-3336579.jpg)
  • Added unittests to cover loading and running from pretrained weights for correct 'kitty' classification
  • Fixed formatting/coding style to comply with ./runtests.sh
  • Updated docs by adding new modules and checking the compiled html

Status

Ready

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.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@wyli wyli requested a review from yiheng-wang-nv April 5, 2021 23:39
wyli
wyli reviewed Apr 6, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

thanks @masadcv! this looks nice I have some minor comments inline, perhaps @yiheng-wang-nv could help double-check with the network details? also please add @skip_if_quick to the unit tests if they requires downloading, e.g. https://github.com/Project-MONAI/MONAI/blob/master/tests/test_ahnet.py#L138

@masadcv
Copy link
Copy Markdown
Contributor Author

masadcv commented Apr 7, 2021 •
edited
Loading

thanks @masadcv! this looks nice I have some minor comments inline, perhaps @yiheng-wang-nv could help double-check with the network details? also please add @skip_if_quick to the unit tests if they requires downloading, e.g. https://github.com/Project-MONAI/MONAI/blob/master/tests/test_ahnet.py#L138

@wyli thank you for your comments! I have addressed all but the drop_connect_rate test. There doesn't seem to be a straightforward way to test it without exposing unnecessary arguments for the class. I have added unittests for drop_connect layer which covers a number of cases. The unittests with pretrained models now use @skip_if_quick as well as test url loading using test_pretrained_networks. We may also want to look into hosting models on a mirror in case we loose the ones from original repo.

'memswish' has been added to unittests for activation.

The kitty test image has been updated to use a Creative Commons image from wikimedia commons.

I need a few more passes to have everything double-checked and in place. I will also run the full coverage integration tests as soon as I have access to relevant resources.

@yiheng-wang-nv would it be possible to have your review in the coming days? Thank you!

yiheng-wang-nv
yiheng-wang-nv reviewed Apr 7, 2021
View reviewed changes
@yiheng-wang-nv
Copy link
Copy Markdown
Contributor

yiheng-wang-nv commented Apr 7, 2021

Thanks @masadcv , I added some comments above.
Hi @wyli , does efficientnet need to support torch script?

@masadcv masadcv force-pushed the 1090-efficientnet-support branch from 928c91d to f2606e7 Compare April 8, 2021 01:29
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Apr 8, 2021

could you please add test_efficientnet to the "min_tests" list of excluding cases

MONAI/tests/min_tests.py

Line 29 in d56c002

exclude_cases = [ # these cases use external dependencies
and fix/remove some of the problematic type hints, I'll make sure this PR is included in v0.5, many thanks @masadcv !

@masadcv
Copy link
Copy Markdown
Contributor Author

masadcv commented Apr 8, 2021

could you please add test_efficientnet to the "min_tests" list of excluding cases

MONAI/tests/min_tests.py

Line 29 in d56c002

exclude_cases = [ # these cases use external dependencies

and fix/remove some of the problematic type hints, I'll make sure this PR is included in v0.5, many thanks @masadcv !

@wyli many thanks! I have done both, locally all tests are passing. It would be awesome to have this in v0.5. Once that is done, I will also add a tutorial. Do you have any suggestions for medical imaging related classification dataset that I can use for a simple tutorial?

@yiheng-wang-nv many thanks for your comments, I have addressed most of your comments. I will look into testing different shapes as you suggested.

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Apr 8, 2021 •
edited
Loading

/black
there's still an error here: https://github.com/Project-MONAI/MONAI/pull/1938/checks?check_run_id=2296192348#step:6:7760

tutorial -- we have a demo script using the IXI dataset (https://brain-development.org/ixi-dataset/) for gender classification https://github.com/Project-MONAI/tutorials/blob/master/3d_classification/torch/densenet_training_array.py perhaps you can make a new example based on it

@masadcv
Copy link
Copy Markdown
Contributor Author

masadcv commented Apr 8, 2021

there's still an error here: https://github.com/Project-MONAI/MONAI/pull/1938/checks?check_run_id=2296192348#step:6:7760

tutorial -- we have a demo script using the IXI dataset (https://brain-development.org/ixi-dataset/) for gender classification https://github.com/Project-MONAI/tutorials/blob/master/3d_classification/torch/densenet_training_array.py perhaps you can make a new example based on it

I am looking into this error now. Should resolve soon.
Thanks for the pointing me in the direction of tutorials.

masadcv added 2 commits April 8, 2021 14:30
@masadcv
fix cuda test error
3d2fa09 
Signed-off-by: masadcv <[email protected]>
@masadcv
WIP fix test errors
410a406 
Signed-off-by: masadcv <[email protected]>
wyli
wyli reviewed Apr 8, 2021
View reviewed changes
wyli
wyli approved these changes Apr 8, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

thanks, it looks nice!

@yiheng-wang-nv
Copy link
Copy Markdown
Contributor

yiheng-wang-nv commented Apr 8, 2021

could you please add test_efficientnet to the "min_tests" list of excluding cases

MONAI/tests/min_tests.py

Line 29 in d56c002

exclude_cases = [ # these cases use external dependencies

and fix/remove some of the problematic type hints, I'll make sure this PR is included in v0.5, many thanks @masadcv !

@wyli many thanks! I have done both, locally all tests are passing. It would be awesome to have this in v0.5. Once that is done, I will also add a tutorial. Do you have any suggestions for medical imaging related classification dataset that I can use for a simple tutorial?

@yiheng-wang-nv many thanks for your comments, I have addressed most of your comments. I will look into testing different shapes as you suggested.

Many thanks @masadcv , this PR looks nice!

@masadcv
Copy link
Copy Markdown
Contributor Author

masadcv commented Apr 8, 2021

Seems to be failing on unrelated test now:
https://github.com/Project-MONAI/MONAI/pull/1938/checks?check_run_id=2297721328

@masadcv masadcv changed the title [WIP] Adding EfficientNetsB0-B7 support Adding EfficientNetsB0-B7 support Apr 8, 2021
@wyli wyli enabled auto-merge (squash) April 9, 2021 00:17
@wyli wyli merged commit e7a7422 into Project-MONAI:master Apr 9, 2021
@masadcv masadcv deleted the 1090-efficientnet-support branch April 9, 2021 09:02
nsrivathsa pushed a commit to nsrivathsa/MONAI that referenced this pull request Apr 12, 2021
@masadcv @yiheng-wang-nv
@wyli @nsrivathsa
Adding EfficientNetsB0-B7 support (Project-MONAI#1938)
5bd8a83 
* adding init efficientnet support

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

* fixing flake8 and further refactoring

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

* adding unittests for efficiennet

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

* making unittests backwards compatible python<3.8

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

* fixed kitty unittests file path

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

* adding docstrings and minor refactoring

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

* fix flake8-py3 failing test

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

* generalize drop_connect for n-dim, fix/add unittests, remove assert

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

* fix failing unittest, CC0-license image for test

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

* refactoring code for review

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

* WIP fix mypy type hint errors

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

* fix cuda test error

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

* WIP fix test errors

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

* adding non-default shape tests

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

* remove 3d case from non-default shape test

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

* refactoring and updating docs

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

Co-authored-by: Yiheng Wang <[email protected]>
Co-authored-by: Wenqi Li <[email protected]>
Signed-off-by: Neha Srivathsa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yiheng-wang-nv yiheng-wang-nv yiheng-wang-nv left review comments

@wyli wyli wyli approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

EfficientNets

3 participants

@masadcv @yiheng-wang-nv @wyli