Instance normalization: enabled affine transformation by default

[dvolgyes]

BatchNorm has a default affine=True, which makes bias term unnecessary, but the InstanceNorm default is affine=False, because of historical reasons. While it is an open issue in Pytorch, see pytorch/pytorch#22755, it would be better to make it explicit. (Let's assume the defaults change, it is better to be explicit. Otherwise line 314 could be also simply changed to "use_bias = not batch_norm" )

Long story short: InstanceNorm needs bias term, and i think the PR is a reasonable solution. (But obviously test it.) In case of accepted PR, a new Pypi release would be nice too.

[fepegar]

I just saw this! Let me take a closer look and resolve the conflicts.

[dvolgyes]

Hi,

I think the fix is trivial, but only you can resolve the conflicts, it requires write access.

[fepegar]

Sorry, I forgot this. Have you tried loading the pretrained model weights with this? As in https://github.com/fepegar/highresnet/blob/4e31ec7021487e651d21d5a7e6f6be75a98a7561/hubconf.py#L30-L31

I'd like the defaults to be compatible with that pretrained model. Wouldn't this commit break compatibility? (I need to write some unit tests for this).

[dvolgyes]

Hi,

I haven't tested, and i have quite limited time at the moment, but my view is:

• instancenorm lacks bias term, and explicitly enabling it would, obviously, be not compatible with cases where there is no bias term. Simply the number of trainable parameters will not be the same.
• i feel that not having bias term would yield inferior results, so it should be a way to have it, maybe not by default, but somehow
• there is a way to convert an existing model, but it might require some work. Technically, a model without bias term is equal with a model which has bias term, but they are all 0.

I did not play with incompatible pytorch models, but it might be that restoring a state from a file only overwrites the parameters, but if there is missing something on the disk, it just leaves the in-memory model intact. Since bias is usually initialized with 0, it might be that simply restoring a model works, but i would make extensive testing.

Long story short:

• it might be accidentally compatible the extended model and the pretrained weights, but most likely not.
• adding bias is widely accepted as necessary condition for good models So i would recommend to:
• add the bias (affine) term
• convert or retrain the model if you want pretrained default

I do not use pretrained models, and i am happy with my own fix in my own fork, so it is more like an upstreaming a bugfix (or at least upstreaming an implementation concern).

[fepegar]

I see. Thanks a lot for the detailed explanation. I'll add a kwarg in a sec.

[fepegar]

Done. Thanks for pointing this out, for the explanation, for the contribution, and for your patience!

Note: although the defaults are theoretically not great, I'll keep the default affine=True so that it is compatible with the parcellation model.