Inconsistent use of handle_legacy_interface() across model builders

[datumbox]

🐛 Describe the bug

We got feedback from some of our downstream frameworks (MMLabs, MobileCV, FastAI, Lightning, etc) that they are not yet ready to pin TorchVision to v0.13 or higher. This means that for compatibility reasons, they are forced to continue using the pretrained=True idiom. For the majority of the models, that's OK because we use the handle_legacy_interface() decorator to set the right weights. Unfortunately not all models support it and thus when they try to initialize the new models they get errors.

For example, the following at MobileCV:

from mobile_cv.model_zoo.models import model_zoo_factory

model_zoo_factory.get_model("swin_t")

Raises an exception:

TypeError: SwinTransformer.__init__() got an unexpected keyword argument 'pretrained'

The use (or lack of use) of the decorator is not consistent. For example in v0.13 we released the efficientnet_v2_s which uses the decorator and the swin_t which doesn't. Similarly shufflenet_v2_x1_5 uses it but resnext101_64x4d doesn't. This lack of consistency across newly introduced models in v0.13 is probably a bug.

Adding it everywhere will ensure the behaviour is aligned across the library and will help the downstream frameworks transition smoother to the new idiom.

Versions

latest main branch

cc @jdsgomes @YosuaMichael

[NicolasHug]

I understand that retroactively supporting pretrained=True helps adoption of 0.13 by offering a more consistent API across all models. But at the same time, supporting pretrained=True on new models isn't really encouraging downstream libraries to migrate - which is something we want them to do.

Does this mean we will have to support pretrained=True for new models that we'll release in 0.14 as well?

[NicolasHug]

Another Q: do I understand correctly that:

• We're adding support for pretrained=True on new model only to give more time and flexibility for downstream libs to migrate
• We're still committed to remove all support for pretrained=True in 0.15. Libraries must have migrated by then.

[datumbox]

Does this mean we will have to support pretrained=True for new models that we'll release in 0.14 as well?

Correct, this would align our approach not only within v0.13 but across all versions. This will also ensure that the downstream libraries can use TorchVision without breakages. I saw you already commented on the PR #6565 that does this.

We're adding support for pretrained=True on new model only to give more time and flexibility for downstream libs to migrate

Correct. The feedback both by internal and external stakeholders was that they didn't want to pin PyTorch (and as a result TorchVision) to the latest version as of yet. They will eventually but this needs to be coordinated. You can see more details by following the external issues and PRs linked to #6365.

We're still committed to remove all support for pretrained=True in 0.15. Libraries must have migrated by then.

I have already issued fixes to the major libraries/frameworks encouraging them to move on. But we certainly don't want to force key players who can't move on the latest version of PyTorch because of CUDA, cloud vendors or other restrictions. I think whether we will remove all support at v0.15 needs to be discussed broadly in our team factoring in feedback from key internal and external stakeholders. Depending on what we will decide, we will update our warnings accordingly. Happy to chat more on our next 1:1.

[NicolasHug]

There are various things to consider here. Easing the adoption of downstream libs is critical, but OTOH we also want to have a clear statement about our deprecation messages and timelines.

This is probably something that should be discussed more broadly with the team, since it directly relates to our deprecation policy. Perhaps this was discussed in yesterday's meeting?