Skip to content
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

[ViT] Graduate ViT from prototype #5173

Merged
merged 8 commits into from
Jan 10, 2022
Merged

[ViT] Graduate ViT from prototype #5173

merged 8 commits into from
Jan 10, 2022

Conversation

yiwen-song
Copy link
Contributor

@yiwen-song yiwen-song commented Jan 6, 2022

Discussed offline with @datumbox, now that all the concerns have been addressed, we can finally graduate ViT models from prototype.

Testing

1. Unittest

EXPECTTEST_ACCEPT=1 pytest test/test_models.py -k ${model_name}

2. Test the legacy weights api

PYTHONPATH=$PYTHONPATH:`pwd` python -u ~/workspace/scripts/run_with_submitit.py --timeout 3000 --ngpus 1 --nodes 1 --partition train --model vit_b_16 --batch-size 1 --test-only --pretrained

Job ID: 14506

3. Test the new weights api in prototype

Run a test job

PYTHONPATH=$PYTHONPATH:`pwd` python -u ~/workspace/scripts/run_with_submitit.py --timeout 3000 --ngpus 1 --nodes 1 --partition train --model vit_b_16 --batch-size 1 --test-only --weights ViT_B_16_Weights.ImageNet1K_V1

Job ID: 14504

cc @datumbox

@facebook-github-bot
Copy link

facebook-github-bot commented Jan 6, 2022

💊 CI failures summary and remediations

As of commit 8d6b0d3 (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sallysyw Looks good. I've added only 2 minor comments, please let me know your thoughts.

Also please add on the PR with the correct tags, so that we can produce easier the release notes. You can read here how tags work.

@datumbox
Copy link
Contributor

datumbox commented Jan 7, 2022

@sallysyw There are a few more things to be done to complete the migration:

The above were omitted earlier as the model was in prototype but now that it graduates, they need to be added.

@yiwen-song
Copy link
Contributor Author

@sallysyw There are a few more things to be done to complete the migration:

The above were omitted earlier as the model was in prototype but now that it graduates, they need to be added.

Sure thing! Let me do it right now.

@yiwen-song
Copy link
Contributor Author

yiwen-song commented Jan 7, 2022

For hubconf.py, linter insists that the import should happen in alphabetical order - which means vit must be put under the segmentation section, any ideas how to avoid that? @datumbox @NicolasHug

@datumbox
Copy link
Contributor

datumbox commented Jan 10, 2022

@sallysyw Everything in that file is in alphabetic order, so adding it below VGG makes sense. If anything I think removed the comments make sense because they are incorrect.

Concerning the unused import, try doing:
# noqa: F401

The rest of the changes look good to me. Ping me when you fix the linters to review and merge. :)

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@yiwen-song yiwen-song merged commit 68f511e into pytorch:main Jan 10, 2022
@yiwen-song yiwen-song deleted the vit branch January 10, 2022 20:16
@github-actions
Copy link

Hey @sallysyw!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

@yiwen-song yiwen-song added new feature other if you have no clue or if you will manually handle the PR in the release notes and removed other if you have no clue or if you will manually handle the PR in the release notes labels Jan 10, 2022
facebook-github-bot pushed a commit that referenced this pull request Jan 17, 2022
Summary:
* graduate vit from prototype

* nit

* add vit to docs and hubconf

* ufmt

* re-correct ufmt

* again

* fix linter

Reviewed By: NicolasHug

Differential Revision: D33618174

fbshipit-source-id: 3a1c6d0915d59069b27ff96a982a337ba9d7690a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants