-
Notifications
You must be signed in to change notification settings - Fork 245
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
Added label names for top 10 confidence categories #175
Conversation
Deploy preview for pytorch-hub-preview ready! Built with commit a6fc887 |
Thanks for the contribution. Ideally we should not need other external libraries to do this. Could you try something like that:
@pmeier / @fmassa: Do we currently have any plans to put the ImageNet categories in TorchVision so that we can avoid the ugly/repetitive step 1? |
Thank you Vasilis. I just made the requested corrections, let me know if that is what you are looking for. |
pytorch_vision_resnext.md
Outdated
|
||
``` | ||
#Download ImageNet labels | ||
!wget https://raw.githubusercontent.com/Tylersuard/hub/master/imagenet_classes.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!wget https://raw.githubusercontent.com/Tylersuard/hub/master/imagenet_classes.txt | |
!wget https://raw.githubusercontent.com/pytorch/hub/master/imagenet_classes.txt |
It's looking good Tyler, thanks a lot for the changes. I added one last comment to update the link of the file to point to pytorch's repo. I'll wait for @pmeier / @fmass 's reply (might take a bit because people started leaving on holidays), to see if there are plans to add the categories directly to TorchVision and avoid downloading them. One last non-blocking comment: I assume you left the ID intentionally in case the user intends to link it back to the ImageNet category? We could optionally format a bit the output to indicate clearly which part is the Id, which is the category and which is the probability but I'll leave it to you how you want to have it. |
I've already asked about this quite some time ago in pytorch/vision#1647, but as it stands @fmassa was against it for licensing reasons. If this has changed, I'm happy to pick it up and adapt the implementation of |
@pmeier Thanks for the reply. I don't know if providing a list with the ImageNet categories has the same issues as redistributing the @Tylersuard Let me check if there is a blocker on adding the list in the notebook repo and I'll get back to you. |
True, but goes in the same direction. Lets wait for @fmassa s judgement. I'm happy to add them if we want that. |
Vasilis, thank you. It does look better without the category number. I switched back to using the CSV and the index disappears. I will update the code later today. |
@pmeier I just checked internally and we are OK adding the list of categories. I think we should start by adding these here and then discuss options for landing them on TorchVision. What do you think? @Tylersuard Thanks Tyler. Do you mind if we completely remove the numbers? Since those ids are not related to the ones that ImageNet uses, there is very little reason to keep them. So if you remove them completely from the |
Ok, I just removed them from the text file. Please note the download link must have the .raw in the URL or it won't work. |
- Remove unnecessary spaces - Coding styles - Simplify code
LGTM @Tylersuard. Thanks for the PR and your patience. To avoid further back and forth and unblock the merge, I pushed directly to your branch a couple of minor changes needed to ensure that the coding style guidelines of PyTorch are met. Please check and let me know if you are OK with them, so that I can merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Much cleaner than my code.
Thank you for accepting the changes! Would you like me to add that same change to the other notebooks?
Sent from ProtonMail Mobile
…On Tue, Dec 22, 2020 at 7:30 AM, Vasilis Vryniotis ***@***.***> wrote:
Merged [#175](#175) into master.
—
You are receiving this because you were mentioned.
Reply to this email directly, [view it on GitHub](#175 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AJ6H6YKRH6H7FQ52HKPSKGLSWC3P7ANCNFSM4VAKWXVA).
|
@Tylersuard Yes, please that would be great! Send a single PR where you apply the same changes that happened on file |
Code will now print out label index, label name, and confidence score for input images.