Improve type hints and docs in timm/models

[a-r-r-o-w]

Part of #1989.

Improves type hints and docs in the following files:

• timm/models/mobilenetv3.py
• timm/models/resnet.py

Fixes minor typo in CONTRIBUTING.md.

@rwightman I would like to know your thoughts on the changes below, and what you do and don't prefer. Will make sure to follow them for subsequent PRs.

I would also like to say that your implementations here have helped me in understanding a lot about going from paper to code as mentioned in the issue. It is really well written and intuitively understandable in terms of code. Thanks for this!

[a-r-r-o-w]

Also noticed that the formatting command black --skip-string-normalization --line-length 120 <path-to-file> (as mentioned in the contributing guide) has not been run on many files. When executed on the above two files, a long list of unrelated indent changes occur, and so I've not committed that so far.

[HuggingFaceDocBuilderDev]

The documentation is not available anymore as the PR was closed or merged.

[rwightman]

@a-r-r-o-w thanks! looking overall good so far. Lettting the CI run, annotations shouldn't impact eager but can impact torchscript tests.

A few things:

Black settings were put in there by another Hugging Face dev, it's a common setup for HF repos but I'm opinionated, black is very opinionated and I have some issues so have not made the leap to turn that on. So let's not run it. Still looking for the right balance for formatting rules, black, autopep8, yapf...

The layer typing as you've noticed isn't super consistent, some models support just nn.Module types, some strings, some both, etc. Looks like you're capturing that, but the 'type' of a non-instance of nn.Module is not nn.Module, it's type[nn.Module] (or typing.Type[nn.Module] for pre python 3.9, which we need to support still, officially 3.7 but can bump up to 3.8 now I feel).

Some of the MobileNetV3 and EfficientNet types are very specific to just those two model classes (BlockArgs especially). Should probably constrain those types to _efficientnet_blocks.py as any other model won't use them. Having the typing module for other common types makes sense but should contain types that are broadly common. Also might want to consider moving that to timm/layers/typing.py as may find some of them propagate to layers with an eventual goal to cover them as well.

Similarly, some other model instances might have some non-trivial types specific to them, unless you see it being common to other models, should probably define at the top of the model module.

[a-r-r-o-w]

Makes sense about the formatter part so will put that on hold.

I've added a few changes based on my interpretation about the typing part, and it makes sense for when I open subsequent PRs for more files. Is this what you meant? If not, could you add suggestions to the relevant parts while reviewing?

Also, don't know how I confused the nn.Module as class instances instead of class objects initially 😅 There are some instances in resnet.py where I thought it was better to put Type[nn.Module] instead of LayerType as the latter is more general (as in Union of types that is then processed by some internal function to yield a nn.Module) and only applies to ResNet (where some preprocessing on the activations is performed) and not BasicBlock (where activations are only instantiated). If this is too convoluted and since typing is not really strict in python, we could just stick to everything being LayerType maybe?

[a-r-r-o-w]

@rwightman Any updates on this?

[rwightman]

@a-r-r-o-w I've had hands full pushing some releases out, handling some other things, on quick look it was looking good but will aim to do a more detailed check and merge in a few days.

[a-r-r-o-w]

Sure, take your time as there's no hurry at all. Eventually it should get faster once I understand the format you expect changes in.

[rwightman]

as with the Type[nn.Module] in the ResNet, this also needs to change right?

This would suffice right?
LayerType = Union[str, Callable, Type[torch.nn.Module]]

[a-r-r-o-w]

Ah, my bad. I forgot the change to Type here.

[rwightman]

I think we don't need the extra type, types.FunctionTYpe, partial, etc either, str + Callable + Type[nn.Module] should cover it all

[a-r-r-o-w]

@rwightman Added changes from your suggestions. Let me know if this is ready to go.