Address feedback for `Fabric.init_module()` (4/4) #17607

awaelchli · 2023-05-11T12:37:01Z

What does this PR do?

Fixes #17581
Partially reverts #17488

Keeps Fabric.init() disentangled from Fabric.init_module()
Renames Fabric.init() to Fabric.init_tensor()

cc @Borda @justusschock @awaelchli @carmocca

for more information, see https://pre-commit.ci

src/lightning/fabric/fabric.py

github-actions · 2023-05-14T15:31:58Z

⚡ Required checks status: All passing 🟢

Groups summary

🟢 pytorch_lightning: Tests workflow

Check ID	Status
pl-cpu (macOS-11, lightning, 3.8, 1.11)	success	✅
pl-cpu (macOS-11, lightning, 3.9, 1.12)	success	✅
pl-cpu (macOS-11, lightning, 3.10, 1.13)	success	✅
pl-cpu (macOS-11, lightning, 3.10, 2.0)	success	✅
pl-cpu (macOS-11, lightning, 3.8, 1.11, oldest)	success	✅
pl-cpu (ubuntu-20.04, lightning, 3.8, 1.11)	success	✅
pl-cpu (ubuntu-20.04, lightning, 3.9, 1.12)	success	✅
pl-cpu (ubuntu-20.04, lightning, 3.10, 1.13)	success	✅
pl-cpu (ubuntu-20.04, lightning, 3.10, 2.0)	success	✅
pl-cpu (ubuntu-20.04, lightning, 3.8, 1.11, oldest)	success	✅
pl-cpu (windows-2022, lightning, 3.8, 1.11)	success	✅
pl-cpu (windows-2022, lightning, 3.9, 1.12)	success	✅
pl-cpu (windows-2022, lightning, 3.10, 1.13)	success	✅
pl-cpu (windows-2022, lightning, 3.10, 2.0)	success	✅
pl-cpu (windows-2022, lightning, 3.8, 1.11, oldest)	success	✅
pl-cpu (macOS-11, pytorch, 3.8, 1.13)	success	✅
pl-cpu (ubuntu-20.04, pytorch, 3.8, 1.13)	success	✅
pl-cpu (windows-2022, pytorch, 3.8, 1.13)	success	✅

These checks are required after the changes to src/lightning/fabric/fabric.py, src/lightning/fabric/strategies/deepspeed.py, src/lightning/fabric/strategies/fsdp.py, src/lightning/fabric/strategies/strategy.py.

🟢 pytorch_lightning: Azure GPU

Check ID	Status
pytorch-lightning (GPUs)	success	✅

These checks are required after the changes to src/lightning/fabric/fabric.py, src/lightning/fabric/strategies/deepspeed.py, src/lightning/fabric/strategies/fsdp.py, src/lightning/fabric/strategies/strategy.py.

🟢 pytorch_lightning: Benchmarks

Check ID	Status
lightning.Benchmarks	success	✅

These checks are required after the changes to src/lightning/fabric/fabric.py, src/lightning/fabric/strategies/deepspeed.py, src/lightning/fabric/strategies/fsdp.py, src/lightning/fabric/strategies/strategy.py.

🟢 fabric: Docs

Check ID	Status
make-doctest (fabric)	success	✅
make-html (fabric)	success	✅

These checks are required after the changes to src/lightning/fabric/fabric.py, src/lightning/fabric/strategies/deepspeed.py, src/lightning/fabric/strategies/fsdp.py, src/lightning/fabric/strategies/strategy.py.

🟢 lightning_fabric: CPU workflow

Check ID	Status
fabric-cpu (macOS-11, lightning, 3.8, 1.11)	success	✅
fabric-cpu (macOS-11, lightning, 3.9, 1.12)	success	✅
fabric-cpu (macOS-11, lightning, 3.10, 1.13)	success	✅
fabric-cpu (macOS-11, lightning, 3.10, 2.0)	success	✅
fabric-cpu (macOS-11, lightning, 3.8, 1.11, oldest)	success	✅
fabric-cpu (ubuntu-20.04, lightning, 3.8, 1.11)	success	✅
fabric-cpu (ubuntu-20.04, lightning, 3.9, 1.12)	success	✅
fabric-cpu (ubuntu-20.04, lightning, 3.10, 1.13)	success	✅
fabric-cpu (ubuntu-20.04, lightning, 3.10, 2.0)	success	✅
fabric-cpu (ubuntu-20.04, lightning, 3.8, 1.11, oldest)	success	✅
fabric-cpu (windows-2022, lightning, 3.8, 1.11)	success	✅
fabric-cpu (windows-2022, lightning, 3.9, 1.12)	success	✅
fabric-cpu (windows-2022, lightning, 3.10, 1.13)	success	✅
fabric-cpu (windows-2022, lightning, 3.10, 2.0)	success	✅
fabric-cpu (windows-2022, lightning, 3.8, 1.11, oldest)	success	✅
fabric-cpu (macOS-11, fabric, 3.8, 1.13)	success	✅
fabric-cpu (ubuntu-20.04, fabric, 3.8, 1.13)	success	✅
fabric-cpu (windows-2022, fabric, 3.8, 1.13)	success	✅

These checks are required after the changes to src/lightning/fabric/fabric.py, src/lightning/fabric/strategies/deepspeed.py, src/lightning/fabric/strategies/fsdp.py, src/lightning/fabric/strategies/strategy.py, tests/tests_fabric/strategies/test_ddp.py, tests/tests_fabric/strategies/test_fsdp_integration.py, tests/tests_fabric/strategies/test_single_device.py, tests/tests_fabric/test_fabric.py.

🟢 lightning_fabric: Azure GPU

Check ID	Status
lightning-fabric (GPUs)	success	✅

These checks are required after the changes to src/lightning/fabric/fabric.py, src/lightning/fabric/strategies/deepspeed.py, src/lightning/fabric/strategies/fsdp.py, src/lightning/fabric/strategies/strategy.py, tests/tests_fabric/strategies/test_ddp.py, tests/tests_fabric/strategies/test_fsdp_integration.py, tests/tests_fabric/strategies/test_single_device.py, tests/tests_fabric/test_fabric.py.

🟢 mypy

Check ID	Status
mypy	success	✅

These checks are required after the changes to src/lightning/fabric/fabric.py, src/lightning/fabric/strategies/deepspeed.py, src/lightning/fabric/strategies/fsdp.py, src/lightning/fabric/strategies/strategy.py.

🟢 install

Check ID	Status
install-pkg (ubuntu-22.04, app, 3.8)	success	✅
install-pkg (ubuntu-22.04, app, 3.10)	success	✅
install-pkg (ubuntu-22.04, fabric, 3.8)	success	✅
install-pkg (ubuntu-22.04, fabric, 3.10)	success	✅
install-pkg (ubuntu-22.04, pytorch, 3.8)	success	✅
install-pkg (ubuntu-22.04, pytorch, 3.10)	success	✅
install-pkg (ubuntu-22.04, lightning, 3.8)	success	✅
install-pkg (ubuntu-22.04, lightning, 3.10)	success	✅
install-pkg (ubuntu-22.04, notset, 3.8)	success	✅
install-pkg (ubuntu-22.04, notset, 3.10)	success	✅
install-pkg (macOS-12, app, 3.8)	success	✅
install-pkg (macOS-12, app, 3.10)	success	✅
install-pkg (macOS-12, fabric, 3.8)	success	✅
install-pkg (macOS-12, fabric, 3.10)	success	✅
install-pkg (macOS-12, pytorch, 3.8)	success	✅
install-pkg (macOS-12, pytorch, 3.10)	success	✅
install-pkg (macOS-12, lightning, 3.8)	success	✅
install-pkg (macOS-12, lightning, 3.10)	success	✅
install-pkg (macOS-12, notset, 3.8)	success	✅
install-pkg (macOS-12, notset, 3.10)	success	✅
install-pkg (windows-2022, app, 3.8)	success	✅
install-pkg (windows-2022, app, 3.10)	success	✅
install-pkg (windows-2022, fabric, 3.8)	success	✅
install-pkg (windows-2022, fabric, 3.10)	success	✅
install-pkg (windows-2022, pytorch, 3.8)	success	✅
install-pkg (windows-2022, pytorch, 3.10)	success	✅
install-pkg (windows-2022, lightning, 3.8)	success	✅
install-pkg (windows-2022, lightning, 3.10)	success	✅
install-pkg (windows-2022, notset, 3.8)	success	✅
install-pkg (windows-2022, notset, 3.10)	success	✅

These checks are required after the changes to src/lightning/fabric/fabric.py, src/lightning/fabric/strategies/deepspeed.py, src/lightning/fabric/strategies/fsdp.py, src/lightning/fabric/strategies/strategy.py.

🟢 link-check

Check ID	Status
check-md-links / markdown-link-check	success	✅

These checks are required after the changes to src/lightning/fabric/CHANGELOG.md.

Thank you for your contribution! 💜

Note
This comment is automatically generated and updates for 60 minutes every 180 seconds. If you have any other questions, contact carmocca for help.

awaelchli · 2023-05-29T22:37:22Z

This is blocking my work on the feature. Please review #17721 and the other parts asap.

for more information, see https://pre-commit.ci

…/revert-17488

codecov · 2023-05-31T15:59:35Z

Codecov Report

Merging #17607 (859306e) into master (e0ce34e) will decrease coverage by 23%.
The diff coverage is 80%.

Additional details and impacted files

@@            Coverage Diff            @@
##           master   #17607     +/-   ##
=========================================
- Coverage      84%      61%    -23%     
=========================================
  Files         419      411      -8     
  Lines       31809    31554    -255     
=========================================
- Hits        26774    19363   -7411     
- Misses       5035    12191   +7156

for more information, see https://pre-commit.ci

…/revert-17488

This reverts commit 614841c.

carmocca

Approving. Discussing further offline

src/lightning/fabric/strategies/deepspeed.py

awaelchli added 5 commits May 7, 2023 05:17

correct changelog

35de489

undo

bedf2b5

deprecate

e265ee6

init

f5b8528

update

5c5e650

github-actions bot added the fabric lightning.fabric.Fabric label May 11, 2023

[pre-commit.ci] auto fixes from pre-commit.com hooks

536aa75

for more information, see https://pre-commit.ci

awaelchli mentioned this pull request May 11, 2023

RFC: Future of init context managers in Fabric #17581

Closed

carmocca reviewed May 11, 2023

View reviewed changes

src/lightning/fabric/fabric.py Show resolved Hide resolved

carmocca reviewed May 11, 2023

View reviewed changes

src/lightning/fabric/fabric.py Outdated Show resolved Hide resolved

awaelchli added the refactor label May 14, 2023

awaelchli added this to the 2.1 milestone May 14, 2023

awaelchli marked this pull request as ready for review May 14, 2023 15:31

awaelchli requested review from justusschock, edenlightning, lantiga and Borda as code owners May 14, 2023 15:31

Merge branch 'master' into fabric/revert-17488

102d707

awaelchli mentioned this pull request May 14, 2023

Support empty weight initialization in Fabric.init_module() #17627

Merged

mergify bot added the has conflicts label May 22, 2023

awaelchli changed the title ~~Address feedback for Fabric.init_module()~~ Address feedback for Fabric.init_module() (4/4) May 29, 2023

awaelchli mentioned this pull request May 30, 2023

Address feedback for Fabric.init_module() (1/4) #17721

Merged

Merge branch 'master' into fabric/revert-17488

4bceacf

mergify bot removed the has conflicts label May 31, 2023

awaelchli and others added 4 commits May 31, 2023 17:10

dedup

869a181

other torch versions

00edba0

update test

d8d7e24

[pre-commit.ci] auto fixes from pre-commit.com hooks

189b085

for more information, see https://pre-commit.ci

awaelchli added 3 commits May 31, 2023 17:34

fix

c5a8bce

Merge remote-tracking branch 'origin/fabric/revert-17488' into fabric…

7ef896b

…/revert-17488

changelog

859306e

justusschock approved these changes May 31, 2023

View reviewed changes

awaelchli and others added 8 commits June 2, 2023 00:57

tests

9f15947

add test

5440fe9

[pre-commit.ci] auto fixes from pre-commit.com hooks

b36aa5c

for more information, see https://pre-commit.ci

update

614841c

Merge remote-tracking branch 'origin/fabric/revert-17488' into fabric…

160ebf4

…/revert-17488

Revert "update"

dbb537d

This reverts commit 614841c.

Merge branch 'master' into fabric/revert-17488

f616103

fix

d7affde

carmocca approved these changes Jun 2, 2023

View reviewed changes

src/lightning/fabric/strategies/deepspeed.py Outdated Show resolved Hide resolved

mergify bot added the ready PRs ready to be merged label Jun 2, 2023

awaelchli added 2 commits June 3, 2023 03:35

add precision context conditionally

355f9c9

Merge branch 'master' into fabric/revert-17488

dabd7c4

mergify bot removed the ready PRs ready to be merged label Jun 3, 2023

awaelchli enabled auto-merge (squash) June 3, 2023 01:37

awaelchli merged commit 67a1479 into master Jun 3, 2023

awaelchli deleted the fabric/revert-17488 branch June 3, 2023 02:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address feedback for `Fabric.init_module()` (4/4) #17607

Address feedback for `Fabric.init_module()` (4/4) #17607

awaelchli commented May 11, 2023 •

edited by github-actions bot

Loading

github-actions bot commented May 14, 2023 •

edited

Loading

awaelchli commented May 29, 2023

codecov bot commented May 31, 2023 •

edited

Loading

carmocca left a comment

Address feedback for Fabric.init_module() (4/4) #17607

Address feedback for Fabric.init_module() (4/4) #17607

Conversation

awaelchli commented May 11, 2023 • edited by github-actions bot Loading

What does this PR do?

github-actions bot commented May 14, 2023 • edited Loading

⚡ Required checks status: All passing 🟢

Groups summary

awaelchli commented May 29, 2023

codecov bot commented May 31, 2023 • edited Loading

Codecov Report

carmocca left a comment

Choose a reason for hiding this comment

Address feedback for `Fabric.init_module()` (4/4) #17607

Address feedback for `Fabric.init_module()` (4/4) #17607

awaelchli commented May 11, 2023 •

edited by github-actions bot

Loading

github-actions bot commented May 14, 2023 •

edited

Loading

codecov bot commented May 31, 2023 •

edited

Loading