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

transformers example #1656

Merged
merged 5 commits into from
Feb 21, 2021
Merged

transformers example #1656

merged 5 commits into from
Feb 21, 2021

Conversation

ahmedo42
Copy link
Contributor

@ahmedo42 ahmedo42 commented Feb 19, 2021

Fixes #960

Description:
Transformers example using huggingface and Ignite , works on TPU & GPU , tried to stick as much as possible to the code in
CIFAR10 example .
Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @ahmedo42 ! Looks good !
I left few comment to improve docs and the code.
Let me try to run it locally and see how it works...

examples/contrib/transformers/README.MD Outdated Show resolved Hide resolved
examples/contrib/transformers/README.MD Outdated Show resolved Hide resolved
examples/contrib/transformers/README.MD Outdated Show resolved Hide resolved
examples/contrib/transformers/README.MD Outdated Show resolved Hide resolved
examples/contrib/transformers/dataset.py Outdated Show resolved Hide resolved
examples/contrib/transformers/main.py Outdated Show resolved Hide resolved
examples/contrib/transformers/main.py Outdated Show resolved Hide resolved
@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 20, 2021

@ahmedo42 I updated a bit more the example (maybe same things should be done on cifar10 as well).
I wonder if you checked the example from your side ? Batch size being 128 is rather large, which GPU do you have :)
Let me know what do you think about these changes. Thanks

@ahmedo42
Copy link
Contributor Author

@ahmedo42 I updated a bit more the example (maybe same things should be done on cifar10 as well).

this definitely is cleaner , I agree that same changes should be done to the CIFAR10 example

I wonder if you checked the example from your side ? Batch size being 128 is rather large, which GPU do you have :)

yes, I checked on GPU , TPU on Kaggle and Colab ( as I Don't have GPU 😄 ) , the default batch size (128) was meant for TPU .

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 21, 2021

Could you please rerun the check from your side to ensure that the example is working. I checked on GPU(s). Maybe, what remains is TPUs...
Once, we are done, we can merge it.

Copy link
Collaborator

@vfdev-5 vfdev-5 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 @ahmedo42 !

@ahmedo42
Copy link
Contributor Author

It works on TPU

@vfdev-5 vfdev-5 merged commit 51970e3 into pytorch:master Feb 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port "bert multi lingual tpu training (8 cores)" to Ignite
2 participants