Add Conv2dNormActivation and Conv3dNormActivation Blocks

[oke-aditya]

Closes #5430

Simple code to try this. It worked fine.

class SimpleModel(torch.nn.Module):
    def __init__(self) -> None:
        super().__init__()
        self.layer1 = Conv3dNormActivation(3, 15, stride=2, padding=2,
                                           activation_layer=torch.nn.LeakyReLU,
                                           inplace=True,
                                           bias=True)
    
    def forward(self, x: torch.Tensor) -> torch.Tensor:
        x = self.layer1(x)
        return x

class EasyModel(torch.nn.Module):
    def __init__(self) -> None:
        super().__init__()
        self.layer1 = Conv2dNormActivation(3, 15, stride=2, padding=2,
                                           activation_layer=torch.nn.LeakyReLU,
                                           inplace=True,
                                           bias=True)
    
    def forward(self, x: torch.Tensor) -> torch.Tensor:
        x = self.layer1(x)
        return x



if __name__ == "__main__":

    image_tensor = torch.rand((5, 3, 224, 224))
    model = EasyModel()
    out = model(image_tensor)
    print(out.size())  # torch.Size([5, 15, 113, 113])

    video_tensor = torch.rand((5, 3, 10, 224, 224))
    model = SimpleModel()
    out = model(video_tensor)
    print(out.size())  # torch.Size([5, 15, 6, 113, 113]

[facebook-github-bot]

💊 CI failures summary and remediations

As of commit 7a6100c (more details on the Dr. CI page):

---

• 2/2 failures introduced in this PR

---

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

binary_win_wheel_py3.10_cu115 (1/1)

Step: "Checkout code" (full log | diagnosis details | 🔁 rerun)

error running git clone "git@github.com:pytorch/vision.git": exit status 128

Writing SSH public key for checkout to "C:\\Users\\circleci\\.ssh\\id_rsa.pub"
Cloning git repository

Connection to github.com closed by remote host.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
error running git clone "git@github.com:pytorch/vision.git": exit status 128

error running git clone "git@github.com:pytorch/vision.git": exit status 128

---

1 failure not recognized by patterns:

Job	Step	Action
cmake_macos_cpu	curl -o conda.sh https://repo.anaconda.com/miniconda/Miniconda3-latest-MacOSX-x86_64.sh
sh conda.sh -b
source $HOME/miniconda3/bin/activate
conda install -yq conda-build cmake
packaging/build_cmake.sh
	🔁 rerun

---

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

[datumbox]

@oke-aditya I've added a few suggestions that aim to simplify the code duplication. Let me know what you think.

All of the proposals assume we will deprecate the old class. There is an alterantive but let's discuss it here after we clean up this proposal.

[datumbox]

Last couple of nits:

[datumbox]

@jdsgomes @oke-aditya The current proposal looks good to me. There is one more pending required here which is to replace all existing ConvNormActivation with Conv2dNormActivation. This should be done on this PR and on next FBsync we should do it also on FBcode.

Finally, I have a question for your consideration. The ConvNormActivation was recently added and yet here we propose its deprecation due to naming convention changes. It's worth assessing how impactful this would be to existing users (aka how many usages we see on Github and on FBcode) before we proceed.

There is an alternative approach that would require minor adaptation of your solution that can allow you to avoid the deprecation. On your current PR, you have the option to rename. _ConvNormActivation to ConvNormActivation, move the conv_layer to the end of the kwargs and set it's default value to torch.nn.Conv2d and you also restore the default value of norm_layer to torch.nn.BatchNorm2d. Everything else remains the same (including keeping 2d and 3d class names). You can also throw a friendly warning on the constructor of ConvNormActivation when we detect that someone called it directly to let them know that the recommended way is to call the 2d version instead (so no deprecation warning, just a nag).

No solution is perfect, each has pros/cons. What you will choose should depend on what you optimize for and how many users you will affect. At any case, please document the reasons here so that it's transparent why we do what we do. 😃

[jdsgomes]

I agree with the comments from @datumbox. I suggested otherwise initially , but the fact is that there are already 39 occurrences of ConvNormActivation internally and 81 occurences of ops.misc import ConvNormActivation in Github (and 374 of ConvNormActivation which might contain similarly named blocks from other implementations). This on its own wont stop from us deprecating it, but together with the fact that is was introduced recently it does’t make sense to deprecated it.

So @oke-aditya agrees, I would suggest to make the changes proposed by @datumbox and we can get it merged!

[jdsgomes]

@oke-aditya raised a good point offline related to this changes. This was recently exposed in ops/init.py and we will be reverting this on a separate PR in the spirit not promoting the usage of ConvNormActivation directly.

So I think I can work on this separately and create a PR that will

• remove ConvNormActivation ops/__init__.py
• change all imports of ops.convNormActivation to ops.misc.ConvNormActivation
• remove all mentions of ConvNormActivation in the docs

[oke-aditya]

So in a brief offline discussion with @jdsgomes

We are exposing ConvNormActivatoin in current release branch
https://github.com/pytorch/vision/blob/release/0.12/torchvision/ops/__init__.py
@jdsgomes will patch this 😄

Also, fully agreeing that PyTorch is highly stable and we need to keep our codebase backward compatible and avoid such sudden changes. (Having the power to deprecate doesn't mean we should keep using it often 😄 )

I will refactor the implementation to be compatible with the old one as @datumbox suggested.

[jdsgomes]

LGTM, thank you for working on this @oke-aditya !

[datumbox]

Suggested change

	warnings.warn("Don't use ConvNormActivation directly. Use Conv2dNormActivation instead.")
	warnings.warn("Don't use ConvNormActivation directly, please use Conv2dNormActivation and Conv3dNormActivation instead.")

[datumbox]

@oke-aditya Looks good!

The work on the block itself is complete but now we need to replace all the usages of ConvNormActivation from the code-base with Conv2dNormActivation. This update needs to happen on this PR to avoid generating unwanted warnings to our users.

[oke-aditya]

Yes I'm on it!!

[oke-aditya]

Please compare 9a5ab98
and https://github.com/pytorch/vision/pull/5440/files

I think I have replaced all the places!

[datumbox]

LGTM, thanks @oke-aditya!

@jdsgomes What you thing of cherrypicking this on the release so that we don't advertise using ConvNormActication on the mobilenets? Did the final RC was produced by the release engineering team?

[jdsgomes]

After discussing offline with @datumbox we decided not to cherrypick this one as we missed the official cut off date and this is quite a bit change.

[jdsgomes]

LGTM!