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

813 ah net #815

Merged
merged 55 commits into from
Aug 4, 2020
Merged

813 ah net #815

merged 55 commits into from
Aug 4, 2020

Conversation

yiheng-wang-nv
Copy link
Contributor

@yiheng-wang-nv yiheng-wang-nv commented Jul 24, 2020

Implemented #813 .

Description

AHNet and its necessary block FCN have been implemented. Based on the author's original implementations ( https://github.com/lsqshr/AH-Net ), one more deterministic version has been added. Specifically, in this version, nn.ConvTranspose3d is used to replace all interpolate and up-sampling manipulations. Moreover, the kernel size for the maxpool layer has been changed to meet the consistency with stride value which is necessary to avoid utilizing atomicAdd operation (an non-deterministic operation).

Several control tests and ablation tests have been done to figure out the non-deterministic places, thus I think my conclusions are reliable. In addition, there are some evidences to support them:

  1. Pytorch's official explanation:
    https://pytorch.org/docs/stable/notes/randomness.html#pytorch

  2. Similar findings in pooling layer's non-deterministic issue:
    https://discuss.pytorch.org/t/inconsistent-results-with-3d-maxpool-on-gpu/38558/2

Since 2D model (FCN) uses resnet50 as its backbone and we do not wanna re-implement it, torchvision package is necessary. Whereas, the version of torchvision and pytorch has to be consistent. In MONAI, we use both Pytorch 1.4 (which needs torchvision 0.5) and the latest version (like Pytorch 1.6 which needs torchvision 0.7), thus my solution to maintain the compatibility is to modify several yml files in .github/workflows (please see the changed files for details). If there are some other more suitable strategies, feel free to mention here and I am able to change it.

What's more, this PR includes some changes for existing codes, including:

  1. Some typing hints for SENet.
  2. Some automatically modified formats by isort.
  3. Re-write dropout layer related codes.
  4. Added inplace=True in relu function in SENet.

All modifications can be traced in the corresponding commits with comments.

As for the performance, I used AHNet (interpolate mode) and follow the notebook examples/notebooks/spleen_segmentation_3d.ipynb to test the performance, the network achieves mean dice > 0.9470 in validation set after 300 epochs. @Nic-Ma

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

yiheng-wang-nv and others added 24 commits July 20, 2020 17:56
The groups parameter is also a parameter in Conv1d (and Conv2d, Conv3d,
...) class in torch.nn.modules.conv.py. In the original code, the value
is set as 1 and cannot be modified directly, which is inconvenient for
some networks such as seresnet and seresnext.

Mention parameters when initialize Convolution

Since one more parameter is added in Cnvolution, the parameter names
should be mentioned (rather than input magic numbers directly) when
initialize this class, otherwise errors (values are input into wrong
parameters) may happen.
The project variable is used to match the number of channels before and
after the sequeeze_and_excitation manipulation. In original code, the
variable may be defined as a Conv layer with kernel size = 1, however,
it can also be defined in other formats, such as a block that is
consisted with a Conv layer and a Norm layer (see downsample part in
github.com/Cadene/pretrained-models.pytorch/blob/master/pretrainedmodels/models/senet.py).
Therefore, a more flexiable parameter could be more convenient.
Type hint losses/ and networks/ forward (Project-MONAI#774)
In a standard senet block and/or resnet block, after the features add
the residuals, one relu layer is also utilized at the end of the block.
Examples can be found in 1) senet implementations
(https://github.com/Cadene/pretrained-models.pytorch/blob/master/pretrainedmodels/models/senet.py),
2) resnet implementations
(https://github.com/pytorch/vision/blob/master/torchvision/models/resnet.py).

Add Bottlenecks for SENet, SEResnet and SEResnext

Bottlenecks are necessary in SE related networks.
SENet, SEResnet and SEResnext networks are implemented.

Unittest for SENet shapes is added

Add typing hints

Some typing hints are added in convolutions.py and squeeze_and_excitation.py
Change the fixed relu function into an optional activation layer.

Re-write test cases for senet

Correct errors for docs and typing hints
For SENet, all relu layers have the parameters inplace equals to
true, thus re-write the corresponding parts.
The differences between nn.Dropout, Dropout2d and Dropout3d are
on the scopes of input tensors. 1D dropout can zero out some
elements for each channel, 2D dropout can zero out some of the
entire 2d channels and 3D dropout can zero out some of the entire
3d channels. Normally, as for SENet (no matter for 2D or 3D), only
1D dropout is needed. Therefore, the original implementations which
keep the dims of dropout layers the same as other layers such as
conv and norm layers may not be very reasonable. Here a new parameter
named dropout_dim with default value 1 is added, and users do not
have to set it unless they need 2d or 3d dropout layers.
FCN is a part of AHNet and helped to train 2D slices first. The
code and the corresponding unittest are added.

Fix errors automatically by isort
Add some typing hints for senet
Pytorch 1.4 corresponds to torchvision==0.5
monai/networks/blocks/fcn.py Outdated Show resolved Hide resolved
@yiheng-wang-nv yiheng-wang-nv requested a review from wyli July 24, 2020 10:49
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Jul 24, 2020

Hi @wyli ,

As we don't use torchvision at other places, can we host the pre-trained model on dropbox to get rid of torchvision?
Thanks.

@wyli
Copy link
Contributor

wyli commented Jul 24, 2020

Hi @wyli ,

As we don't use torchvision at other places, can we host the pre-trained model on dropbox to get rid of torchvision?
Thanks.

looks like the current code requires both a .pth file and a python script to define the network https://github.com/pytorch/vision/blob/master/torchvision/models/resnet.py
if that's the case then we should use torchvision for now, we don't want to reimplement torchvision's resnet.py

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Jul 24, 2020

OK, sounds good.
@yiheng-wang-nv , please update to use optional import as @wyli suggested.

Thanks.

@yiheng-wang-nv yiheng-wang-nv changed the title [WIP] 813 ah net 813 ah net Jul 31, 2020
@yiheng-wang-nv yiheng-wang-nv marked this pull request as ready for review July 31, 2020 06:55
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Aug 3, 2020

Thanks for the update.
Hi @wyli and @ericspod ,

Could you please help review it?
Thanks.

@yiheng-wang-nv
Copy link
Contributor Author

yiheng-wang-nv commented Aug 3, 2020

Thanks for the update.
Hi @wyli and @ericspod ,

Could you please help review it?
Thanks.

Thanks Nic, Wenqi and Eric. I've updated the description to roughly explain all the changed files. If there are any issues, feel free to contact me, thanks!

Copy link
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 good, i have some minor comments inline

.github/workflows/pythonapp.yml Show resolved Hide resolved
monai/networks/blocks/convolutions.py Show resolved Hide resolved
monai/networks/blocks/convolutions.py Show resolved Hide resolved
monai/networks/blocks/fcn.py Outdated Show resolved Hide resolved
monai/networks/blocks/fcn.py Outdated Show resolved Hide resolved
monai/networks/nets/ahnet.py Outdated Show resolved Hide resolved
monai/networks/blocks/fcn.py Outdated Show resolved Hide resolved
monai/networks/nets/ahnet.py Outdated Show resolved Hide resolved
monai/networks/nets/ahnet.py Outdated Show resolved Hide resolved
Copy link
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.

looks nice, thanks @yiheng-wang-nv

@wyli
Copy link
Contributor

wyli commented Aug 4, 2020

/black

@Nic-Ma Nic-Ma merged commit f83aaaf into Project-MONAI:master Aug 4, 2020
@wyli wyli mentioned this pull request Aug 4, 2020
@yiheng-wang-nv yiheng-wang-nv deleted the 813-AHNet branch August 4, 2020 23:13
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.

None yet

4 participants