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

Adding Minimal Reproducible Usage Example For TPU support on examples/seq2seq #5960

Closed

Conversation

AdityaSoni19031997
Copy link
Contributor

@AdityaSoni19031997 AdityaSoni19031997 commented Jul 22, 2020

Attempt to resolve #5895.

Minimal Working Colab Example.

For using more than a single core, one needs to ensure that enough RAM is available else wait for PyTorch-XLA to release a stable version. They have also released a fix way back that prevent excessive memory usage for nightly.

@AdityaSoni19031997
Copy link
Contributor Author

The test will obviously break, right?

@marton-avrios
Copy link

Any progress on merging this?

@AdityaSoni19031997
Copy link
Contributor Author

they won't accept it as the checks have failed?
But it's expected for the checks to fail as I have modified modeling_bart and it's gonna have one more Param count.

@marton-avrios
Copy link

Is that so @sshleifer?

@@ -943,6 +943,7 @@ def __init__(self, config: BartConfig):
super().__init__(config)
base_model = BartModel(config)
self.model = base_model
self.lm_head = _make_linear_from_emb(self.model.shared)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to call this again after line 952 to make the tests pass.

@sshleifer
Copy link
Contributor

Thanks for the contribution, this looks awesome!

We can't merge with failing tests, but I think the tests can pass.

Could you also check

RUN_SLOW=1 pytest tests/test_modeling_bart.py
RUN_SLOW=1 pytest tests/test_modeling_marian.py
RUN_SLOW=1 pytest test_modeling_mbart.py

add the USE_CUDA=1 prefix to make them run faster on GPU.

@sshleifer
Copy link
Contributor

Actually can we add a support_tpu flag to BartConfig, init it to False, and only allocate lm_head if it's set to True. I'm concerned that we are wasting RAM when we train on GPU. (I would happily change my mind if I encountered evidence that this change doesn't change GPU RAM consumption.)

@marton-avrios
Copy link

marton-avrios commented Jul 27, 2020

I tried this version and it seems to work but it stucks at "Validation sanity check". Working colab here

@AdityaSoni19031997
Copy link
Contributor Author

AdityaSoni19031997 commented Jul 27, 2020 via email

@stale
Copy link

stale bot commented Sep 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Sep 26, 2020
@sshleifer
Copy link
Contributor

sshleifer commented Sep 26, 2020

This is now supported by Seq2SeqTrainer. Use that if you want TPU support!

@sshleifer sshleifer closed this Sep 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

examples/seq2seq/finetune.py and BART supports TPU
3 participants