-
Notifications
You must be signed in to change notification settings - Fork 7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add efficientnetv2 #4910
add efficientnetv2 #4910
Conversation
💊 CI failures summary and remediationsAs of commit ad5a240 (more details on the Dr. CI page):
🕵️ 1 new failure recognized by patternsThe following CI failures do not appear to be due to upstream breakages: binary_libtorchvision_ops_android (1/1)Step: "Build" (full log | diagnosis details | 🔁 rerun)
|
Job | Step | Action |
---|---|---|
python_lint | pre-commit run --all-files | 🔁 rerun |
This comment was automatically generated by Dr. CI (expand for details).
Please report bugs/suggestions to the (internal) Dr. CI Users group.
I implement EfficientNetV2 just like the style of EfficientNetV1 in torchvision models. I also converted the timm pretrained weights, but I found the acc dropped a lot (for example, EfficientNetV2-S is about 82.9 rather 83.9) because of the different padding for 3x3 conv of stride2. |
@xiaohu2015 First of all let me confirm that the EfficientNetV2 is within the list of models we would like to include on TorchVision. So the contribution is definitely useful and thank you for it. Moreover your code is of high quality and follows the conventions/style of TorchVision. Unfortunately accepting model contributions from the community has been challenging in the past, primarily due to training issues (see our contribution guide). You can read more about this at #2707 (comment). The TLDR for it is that we don't accept contributions without trained weights which fully reproduce the accuracy of the paper. On the one hand, getting the weights is quite a lot to ask from open-source contributors because training is quite expensive. On the other, a contribution without weights means that an internal maintainer needs to do a significant amount of work to reproduce the paper and that might not be possible due to different priorities. Taking weights from elsewhere is something done in the past but we usually try to avoid for various reasons (lack of reproducibility, minor implementation differences can lead to large accuracy gaps as the ones you report above, adds little value to the community etc). This is why especially for very large features such as this, it's imperative that we discuss the details prior starting the work. Here are a few things we want to typically clarify:
Doing the above help us identify the risks, plan features based on our capacity and ensure we don't throwaway work. I've been an Open-source contributor myself in the past and there is nothing worst than spending couple of weeks to write a feature only to be rejected or to remain unmerged for months due to lack of priority... This is the reason we want to clarify all the above prior starting writing the PR. Concerning the contents of this PR, here are a few concerns:
I think to make this PR mergeable will require a lot of work, so I would like to understand the situation on your side. :) |
@datumbox thanks, I am looking foward to your implemention. I will close this pr. |
@xiaohu2015 Your PRs are definitely welcome. If you have any other features you would like to work on let's chat about it on their linked issues to map out the details. :) |
@datumbox Ok, thanks again. |
Thank you @xiaohu2015 for this great implementation! |
@bratao you can get the original implementation here: https://github.com/xiaohu2015/nncls/blob/main/models/efficientnetv2.py |
@xiaohu2015 We should consider adapting the existing efficientnet.py implementation of TorchVision to support both V1 and V2 and port it in. This can wait for after your other PR #4961 about FCOS is merged. :) |
I want to add efficientnetv2. #2707