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

Accelerator Refactor: Precision Plugins #5718

Merged
merged 14 commits into from
Jan 31, 2021
Merged

Accelerator Refactor: Precision Plugins #5718

merged 14 commits into from
Jan 31, 2021

Conversation

justusschock
Copy link
Member

What does this PR do?

Adds precision plugins from #5616
To be merged after #5715

Co-Authored with @awaelchli

@justusschock justusschock added this to the 1.2 milestone Jan 30, 2021
@justusschock justusschock self-assigned this Jan 30, 2021
@pep8speaks
Copy link

pep8speaks commented Jan 30, 2021 •
edited
Loading

Hello @justusschock! Thanks for updating this PR.

Line 26:121: E501 line too long (136 > 120 characters)
Line 40:121: E501 line too long (139 > 120 characters)

Comment last updated at 2021-01-31 17:46:16 UTC

@justusschock justusschock mentioned this pull request Jan 30, 2021
Merged
@Borda Borda requested a review from ananthsub January 30, 2021 20:01
@Borda Borda mentioned this pull request Jan 30, 2021
Merged
12 tasks
ananthsub
ananthsub reviewed Jan 30, 2021
View reviewed changes
pytorch_lightning/accelerators/accelerator.py Outdated Show resolved Hide resolved
pytorch_lightning/accelerators/accelerator.py Outdated Show resolved Hide resolved
pytorch_lightning/accelerators/accelerator.py Outdated
Comment on lines 25 to 38
- CPU
- GPU
Copy link
Contributor

Choose a reason for hiding this comment

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

@SeanNaren if we want to do ZeRO-v3 and offload more to CPU memory, is that handled within the training type plugin entirely? is there any dependency on the accelerator in which the plugin is contained?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah handled explicitly within the training type plugin, or more technically, within the sharded DDP/OSS class itself. I think from what I recall, an offload_device will be exposed giving the user control (in this case, our accelerator control)

pytorch_lightning/plugins/precision/precision_plugin.py Outdated Show resolved Hide resolved
pytorch_lightning/plugins/training_type/training_type_plugin.py Outdated Show resolved Hide resolved
SeanNaren
SeanNaren reviewed Jan 31, 2021
View reviewed changes
pytorch_lightning/plugins/precision/tpu_bfloat.py
from pytorch_lightning.plugins.precision.precision_plugin import PrecisionPlugin


class TPUHalfPrecisionPlugin(PrecisionPlugin):
Copy link
Contributor

Choose a reason for hiding this comment

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

ooo I didn't see this, this is nice :)

justusschock and others added 11 commits January 31, 2021 17:23
@justusschock
add basic accelerator class.
a6e6e20 
Co-Authored with @awaelchi
@justusschock
add basic trainign type plugin.
3c5b0a6 
Co-Authored with @awaelchi
@justusschock
pep8
5ab4b9e 
Co-authored-by: @awaelchi
@justusschock @awaelchli
update copyright
ab4660d 
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
@justusschock @awaelchli
add apex_amp
1ade664 
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
@justusschock @awaelchli
add mixed base class
bcd0e7a 
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
@justusschock @awaelchli
add native amp
a287567 
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
@justusschock @awaelchli
add native amp sharded
1ff9b42 
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
@justusschock @awaelchli
add tpu bfloat
1b82554 
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
@justusschock @awaelchli
add inits
791fe19 
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
@justusschock
Update precision_plugin.py
94e0b28
@mergify mergify bot removed the has conflicts label Jan 31, 2021
@justusschock justusschock enabled auto-merge (squash) January 31, 2021 16:33
Borda
Borda approved these changes Jan 31, 2021
View reviewed changes
Copy link
Member

@Borda Borda left a comment

Choose a reason for hiding this comment

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

lets fix import...

pytorch_lightning/plugins/precision/mixed.py Show resolved Hide resolved
pytorch_lightning/plugins/base_plugin.py Outdated Show resolved Hide resolved
justusschock and others added 3 commits January 31, 2021 18:17
@justusschock @Borda
Update pytorch_lightning/plugins/base_plugin.py
97eab50 
Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
@justusschock
fix imports
de8fe1b
@justusschock
Merge branch 'ref/precision_plugins' of github.com:PyTorchLightning/p…
60445ad 
…ytorch-lightning into ref/precision_plugins
@codecov
Copy link

codecov bot commented Jan 31, 2021 •
edited
Loading

Codecov Report

Merging #5718 (60445ad) into release/1.2-dev (3bacac7) will decrease coverage by 0%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##           release/1.2-dev   #5718    +/-   ##
================================================
- Coverage               89%     89%    -0%     
================================================
  Files                  173     173            
  Lines                12495   12339   -156     
================================================
- Hits                 11175   11017   -158     
- Misses                1320    1322     +2

@justusschock justusschock merged commit 069ae27 into release/1.2-dev Jan 31, 2021
@justusschock justusschock deleted the ref/precision_plugins branch January 31, 2021 18:12
@Borda Borda added the ready PRs ready to be merged label Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ananthsub ananthsub ananthsub left review comments

@Borda Borda Borda approved these changes

@awaelchli awaelchli awaelchli approved these changes

@SeanNaren SeanNaren SeanNaren approved these changes

@tchaton tchaton Awaiting requested review from tchaton

@williamFalcon williamFalcon Awaiting requested review from williamFalcon

Assignees

@justusschock justusschock

Labels
ready PRs ready to be merged refactor
Projects
None yet
Milestone
pl:1.2
Development

Successfully merging this pull request may close these issues.
6 participants
@justusschock @pep8speaks @ananthsub @awaelchli @Borda @SeanNaren