-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Add Swin Transformer Example #1346
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for pytorch-examples-preview canceled.
|
swin_transformer/swin_transformer.py
Outdated
use_cuda = torch.cuda.is_available() | ||
device = torch.device("cuda" if use_cuda else "cpu") |
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.
It would be better to use the accelerator
API
use_cuda = torch.cuda.is_available() | |
device = torch.device("cuda" if use_cuda else "cpu") | |
use_accel = torch.accelerator.is_available() | |
device = torch.accelerator.current_accelerator() if use_accel else torch.device("cpu") | |
print(f"Using device: {device}") |
swin_transformer/README.md
Outdated
Install dependencies: | ||
|
||
```bash | ||
pip install torch torchvision |
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.
Use requirements.txt
to be consistent with other examples
swin_transformer/README.md
Outdated
Testing is done automatically after each epoch. To only test, run with: | ||
|
||
```bash 8000 | ||
python swin_transformer.py --epochs 0 |
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.
I guess this should be 1
epoch
@jafraustro , Thanks for the review. I've updated my PR. Please have a look and lemme know if something else is needed. :)) |
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.
LGTM
@jafraustro can this be merged ? or does it require something else from my end? |
Hi @msaroufim, could you give a look to this PR? |
swin_transformer/requirements.txt
Outdated
@@ -0,0 +1,2 @@ | |||
torch |
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.
torch | |
torch>=2.6 |
Due to usage of torch.accelerator
.
run_python_examples.sh
Outdated
@@ -192,6 +196,7 @@ function stop() { | |||
word_language_model/model.pt \ | |||
gcn/cora/ \ | |||
gat/cora/ || error "couldn't clean up some files" | |||
swin_transformer/swin_cifar10.pt || error "couldn't clean up some files" |
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.
It will be failing with "command swin_transformer/swin_cifar10.pt" not found. You need to add to the list. I can't suggest the change since it touches non-modified cmdline, but code should be:
gat/cora/ \
swin_transformer/swin_cifar10.pt || error "couldn't clean up some files"
I.e. line break on gat/cora/
line.
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.
yep, i ran the examples and yes I had to also change the swin to swin_transformer in Line 228
swin_transformer/swin_transformer.py
Outdated
@@ -0,0 +1,207 @@ | |||
from __future__ import print_function |
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.
It seems script does not actually use print_function
. Can this be dropped?
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.
fixed in 5478049
swin_transformer/swin_transformer.py
Outdated
torch.save(model.state_dict(), "swin_cifar10.pt") | ||
|
||
if __name__ == '__main__': | ||
main() |
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.
main() | |
main() | |
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.
fixed in 5478049
swin_transformer/README.md
Outdated
### Save the model | ||
|
||
```bash | ||
python swin_transformer.py --save-model |
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.
Is --save-model
really needed? should not the trained model be always saved?
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.
done! fixes in 5478049
@dvrogozh Thanks for your reviwes, I've made the changes :) |
Solves part of #1131
@msaroufim