Remove efficientnet-pytorch dependency #1018
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
|
Hey @adamjstewart! Thanks for working on it! Yeah, in order to remove it we have to ensure backward compatibility with existing timm backbones + add an aliases, otherwise we can copy-paste existing code directly to the lib or define a weight-loading hook |
|
Let me see if efficientnet-pytorch weights can be loaded into a timm backbone. If not, would that be a deal breaker? I.e., if the API remains the same (no code changes required) but the weights change from efficientnet-pytorch to timm (thus breaking reproducibility) would that be an issue? |
|
Yes, I don't want some users to have issues with weights loading while they update to a newer version. EfficientNet especially, because it is a very popular backbone. On our side, there were no issues with this library, so I don't feel like we need to cause inconvenience just to remove the dependency. The solution might be to maintain it on our side (copy-paste code) or introduce a load hook. |
|
Maintaining it on our side would also be fine with me, although I would still like to deprecate it in favor of timm in the long run. |
|
Deprecation in the long run is fine with me as well 👍 |
|
Tried investigating this but the timm and efficientnet-pytorch implementations are sufficiently different that I don't think we can easily remap the keys. I think we should simply deprecate efficientnet-pytorch (and possibly vendor it). Thoughts? How long would you be comfortable waiting before removing? I usually wait one minor release, so remove it in 0.5.0. Not sure what release cadence you would like, I usually do a minor release every 6 months so I can add new features more often. |
|
Let's move to our code-base, efficientnet seems to be a popular architecture, so I am not sure we can deprecate it quickly, I would better maintain it for a while and force users to use |
|
I am going to move all encoder checkpoints to the HF Hub. This will allow us to see the download statistics and potentially deprecate them more aggressively if there is no usage, or wait until usage decreases, prioritizing the timm-universal encoder. Although it does not solve the problem entirely, as there is no way to see how many pre-trained checkpoints users already have. Also it might be that the ported version can be a bit more optimized for depths less than 5. This is because the later stages are not executed for the ported versions, as opposed to timm. |
|
Do you want to open a PR to vendor efficientnet-pytorch (and possibly pretrainedmodels)? Or do we just deprecate them and wait until HF Hub download metrics taper off? I've never vendored a dep before so not sure what code/license changes are required. Efficientnet-pytorch is Apache-2.0 and pretrainedmodels is BSD-3-Clause. |
|
No pull requests are needed, I just want to replace download links in our source code. The weight loading is managed on our side. Regarding the license: I suppose we can just add a header to the files containing the code, for example, for pretrainedmodels.pytorch: Additionally, add a note in our License folder. |
|
Where would the vendored code live? How would it be imported? Do you want a |
|
It's better to put it in the |
|
Will wait until #1031 is merged and then try vendor these in a separate PR. Do you want the entire source code of https://github.com/lukemelas/EfficientNet-PyTorch/tree/master/efficientnet_pytorch in |
|
Ok, lets put it into |
I first mentioned this in #918, but we should really remove efficientnet-pytorch for the following reasons:
If we need to ensure backwards compatibility, we can add aliases to the timm implementations.
P.S. I'm also planning on doing this for as many of our other dependencies as I can. I'll wait to see if this PR gets any traction before starting on pretrainedmodels next.