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.

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

Enable TPU support #868

Merged
merged 90 commits into from
Feb 17, 2020
Merged

Enable TPU support #868

merged 90 commits into from
Feb 17, 2020

Conversation

williamFalcon
Copy link
Contributor

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
Member

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
Member

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

this was likely from rebase...

@Borda
Copy link
Member

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

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

@dlibenzi dlibenzi dlibenzi left review comments

@luiscape luiscape luiscape left review comments

@Borda Borda Awaiting requested review from Borda Borda is a code owner

@jeffling jeffling Awaiting requested review from jeffling

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