Repository navigation
475 add different downsampling methods to patch gan discriminator - #479
Conversation
|
@StijnvWijn thanks for fixing the tutorial. Since the tutorial file has undergone a lot of changes, could you create a different issue and PR for this? It's a different issue from the one treated in this one, so we would like to treat them separately. |
|
In addition, could you add use cases to test test_patch_gan.py that uses the available pooling methods? No additional test is needed, just additional configurations in TEST_3D and TEST_2D. |
There was a problem hiding this comment.
Thank you for addressing this PR to enable having pooling layers in the Multi-scale PatchGAN discriminator. Prior to approve the PR, the following changes need to be performed on the code:
- No support for 3D; pooling layers should be defined using get_pool_layer from monai.networks.layers, to take in spatial_dims.
- Due to not using this function, the padding behaviour is wrong. The default should be "same" padding. This will probably get fixed addressing the previous point.
- At the moment, setting the pooling layers as an attribute is causing the forward method not to work well. It should not be set as an attribute.
- Use cases should be added to test_patch_gan.py to make sure the codebase works with pooling_method != None.
- The changes to the SPADE tutorial to make it work on Windows should be a separate PR as it is an entirely different issue.
c1e1225 to
7a90fb8
Compare
virginiafdez
left a comment
There was a problem hiding this comment.
Hello,
Thanks very much for applying all the changes! Everything looks good now! The only thing is adding the padding on the pooling layer creation because apparently, it's 0 by default.
Once this is added, we can approve the PR!
|
Things work, I ran the autofix script and committed these changes. |
virginiafdez
left a comment
There was a problem hiding this comment.
Ran autofix on top of the changes.
Added features according to issue #475 leaving the default behaviour the same. According to my testing it should be as fast as the previous implementation.
Additionally, I fixed the 2D_spade_VAE.py tutorial, as it didn't work on my Windows laptop.