Skip to content

Enable TPU support #868

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.

Sign up for GitHub

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

Jump to bottom
Merged
williamFalcon merged 90 commits into from
Feb 17, 2020
Merged

Enable TPU support #868

williamFalcon merged 90 commits into from
Feb 17, 2020

Conversation

@williamFalcon
Copy link
Contributor

@williamFalcon williamFalcon commented Feb 16, 2020

Enabling TPU support for pl

@williamFalcon williamFalcon changed the title added tpu support [WIP] Enable TPU support [WIP] Feb 16, 2020
@pep8speaks
Copy link

pep8speaks commented Feb 16, 2020 •
edited
Loading

Hello @williamFalcon! Thanks for updating this PR.

Line 86:101: E501 line too long (101 > 100 characters)

Line 860:101: E501 line too long (106 > 100 characters)

Comment last updated at 2020-02-17 16:16:44 UTC

@williamFalcon williamFalcon merged commit d4a31f0 into master Feb 17, 2020
@Borda
Copy link
Collaborator

Borda commented Feb 17, 2020

well, with master merged (yielding change in 23 files) it is hard to find what is TPU change :{

@williamFalcon
Copy link
Contributor Author

williamFalcon commented Feb 17, 2020 •
edited
Loading

yeah... it's a big feature touching almost all parts of the code. nothing i can do about it...

@williamFalcon williamFalcon deleted the tpu branch February 17, 2020 21:08
@Borda
Copy link
Collaborator

Borda commented Feb 17, 2020

I suspect that you have included several past PRs, I see for example entire CHANGELOG, adding segmentation example which I guess was not part it TPU support... @luiscape ?

@williamFalcon
Copy link
Contributor Author

williamFalcon commented Feb 17, 2020

this was likely from rebase...

@Borda
Copy link
Collaborator

Borda commented Feb 17, 2020

this was likely from rebase...

git merge master not git rebase master but happy to have it landed lol

@williamFalcon williamFalcon mentioned this pull request Feb 21, 2020
Closed
dlibenzi
dlibenzi reviewed Feb 21, 2020
View reviewed changes
pytorch_lightning/trainer/training_loop.py
model.optimizer_step(self.current_epoch, batch_idx,
optimizer, opt_idx, optimizer_closure)
if self.use_tpu:
xm.optimizer_step(optimizer, barrier=True)
Copy link

@dlibenzi dlibenzi Feb 21, 2020

Choose a reason for hiding this comment

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

If you use the ParallelLoader, you do not need barrier=True here.

@Borda Borda added the feature Is an improvement or enhancement label Mar 3, 2020
@Borda Borda added this to the 0.7.0 milestone Mar 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Borda Borda Awaiting requested review from Borda

@jeffling jeffling Awaiting requested review from jeffling

2 more reviewers

@luiscape luiscape luiscape left review comments

@dlibenzi dlibenzi dlibenzi left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

feature Is an improvement or enhancement

Projects

None yet

Milestone

pl:0.7

Development

Successfully merging this pull request may close these issues.

8 participants

@williamFalcon @pep8speaks @Borda @luiscape @dlibenzi @akshaykulkarni07 @ethanwharris @xssChauhan