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

Avoid modifying the default dtype on exception #18500

Merged
merged 8 commits into from
Sep 14, 2023

Conversation

carmocca
Copy link
Contributor

@carmocca carmocca commented Sep 7, 2023

What does this PR do?

Since torch.set_default_dtype modifies the global state, we need to make sure that the default dtype is restored if an exception happens while we are modifying it.

An example where this is a problem is tests, if one test fails, the default dtype will be different for others, possibly creating a chain of failures

cc @Borda @carmocca @justusschock @awaelchli

@carmocca carmocca added bug Something isn't working fabric lightning.fabric.Fabric plugin pl Generic label for PyTorch Lightning package labels Sep 7, 2023
@carmocca carmocca added this to the 2.1 milestone Sep 7, 2023
@carmocca carmocca self-assigned this Sep 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

⚡ 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
pl-cpu (macOS-12, pytorch, 3.11, 2.0) success
pl-cpu (ubuntu-22.04, pytorch, 3.11, 2.0) success
pl-cpu (windows-2022, pytorch, 3.11, 2.0) success

These checks are required after the changes to src/lightning/fabric/plugins/precision/deepspeed.py, src/lightning/fabric/plugins/precision/double.py, src/lightning/fabric/plugins/precision/fsdp.py, src/lightning/fabric/plugins/precision/half.py, src/lightning/fabric/plugins/precision/transformer_engine.py, src/lightning/pytorch/plugins/precision/deepspeed.py, src/lightning/pytorch/plugins/precision/double.py, src/lightning/pytorch/plugins/precision/fsdp.py, src/lightning/pytorch/plugins/precision/half.py, tests/tests_pytorch/plugins/precision/test_all.py.

🟢 pytorch_lightning: Azure GPU
Check ID Status
[pytorch-lightning (GPUs) (testing Lightning latest)](https://dev.azure.com/Lightning-AI/72ab7ed8-b00f-4b6e-b131-3388f7ffafa7/_build/results?buildId=174517&view=logs&jobId=47e66f3c-897a-5428-da11-bf5c7745762e) success
[pytorch-lightning (GPUs) (testing PyTorch latest)](https://dev.azure.com/Lightning-AI/72ab7ed8-b00f-4b6e-b131-3388f7ffafa7/_build/results?buildId=174517&view=logs&jobId=3f274fac-2e11-54ca-487e-194c91f3ae9f) success

These checks are required after the changes to src/lightning/pytorch/plugins/precision/deepspeed.py, src/lightning/pytorch/plugins/precision/double.py, src/lightning/pytorch/plugins/precision/fsdp.py, src/lightning/pytorch/plugins/precision/half.py, tests/tests_pytorch/plugins/precision/test_all.py, src/lightning/fabric/plugins/precision/deepspeed.py, src/lightning/fabric/plugins/precision/double.py, src/lightning/fabric/plugins/precision/fsdp.py, src/lightning/fabric/plugins/precision/half.py, src/lightning/fabric/plugins/precision/transformer_engine.py.

🟢 pytorch_lightning: Benchmarks
Check ID Status
lightning.Benchmarks success

These checks are required after the changes to src/lightning/fabric/plugins/precision/deepspeed.py, src/lightning/fabric/plugins/precision/double.py, src/lightning/fabric/plugins/precision/fsdp.py, src/lightning/fabric/plugins/precision/half.py, src/lightning/fabric/plugins/precision/transformer_engine.py, src/lightning/pytorch/plugins/precision/deepspeed.py, src/lightning/pytorch/plugins/precision/double.py, src/lightning/pytorch/plugins/precision/fsdp.py, src/lightning/pytorch/plugins/precision/half.py.

🟢 fabric: Docs
Check ID Status
docs-make (fabric, doctest) success
docs-make (fabric, html) success

These checks are required after the changes to src/lightning/fabric/plugins/precision/deepspeed.py, src/lightning/fabric/plugins/precision/double.py, src/lightning/fabric/plugins/precision/fsdp.py, src/lightning/fabric/plugins/precision/half.py, src/lightning/fabric/plugins/precision/transformer_engine.py.

🟢 pytorch_lightning: Docs
Check ID Status
docs-make (pytorch, doctest) success
docs-make (pytorch, html) success

These checks are required after the changes to src/lightning/pytorch/plugins/precision/deepspeed.py, src/lightning/pytorch/plugins/precision/double.py, src/lightning/pytorch/plugins/precision/fsdp.py, src/lightning/pytorch/plugins/precision/half.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
fabric-cpu (macOS-12, fabric, 3.11, 2.0) success
fabric-cpu (ubuntu-22.04, fabric, 3.11, 2.0) success
fabric-cpu (windows-2022, fabric, 3.11, 2.0) success

These checks are required after the changes to src/lightning/fabric/plugins/precision/deepspeed.py, src/lightning/fabric/plugins/precision/double.py, src/lightning/fabric/plugins/precision/fsdp.py, src/lightning/fabric/plugins/precision/half.py, src/lightning/fabric/plugins/precision/transformer_engine.py, tests/tests_fabric/plugins/precision/test_all.py, tests/tests_fabric/test_connector.py.

🟢 lightning_fabric: Azure GPU
Check ID Status
[lightning-fabric (GPUs) (testing Fabric latest)](https://dev.azure.com/Lightning-AI/72ab7ed8-b00f-4b6e-b131-3388f7ffafa7/_build/results?buildId=174519&view=logs&jobId=3f274fac-2e11-54ca-487e-194c91f3ae9f) success
[lightning-fabric (GPUs) (testing Lightning latest)](https://dev.azure.com/Lightning-AI/72ab7ed8-b00f-4b6e-b131-3388f7ffafa7/_build/results?buildId=174519&view=logs&jobId=47e66f3c-897a-5428-da11-bf5c7745762e) success

These checks are required after the changes to src/lightning/fabric/plugins/precision/deepspeed.py, src/lightning/fabric/plugins/precision/double.py, src/lightning/fabric/plugins/precision/fsdp.py, src/lightning/fabric/plugins/precision/half.py, src/lightning/fabric/plugins/precision/transformer_engine.py, tests/tests_fabric/plugins/precision/test_all.py, tests/tests_fabric/test_connector.py.

🟢 mypy
Check ID Status
mypy success

These checks are required after the changes to src/lightning/fabric/plugins/precision/deepspeed.py, src/lightning/fabric/plugins/precision/double.py, src/lightning/fabric/plugins/precision/fsdp.py, src/lightning/fabric/plugins/precision/half.py, src/lightning/fabric/plugins/precision/transformer_engine.py, src/lightning/pytorch/plugins/precision/deepspeed.py, src/lightning/pytorch/plugins/precision/double.py, src/lightning/pytorch/plugins/precision/fsdp.py, src/lightning/pytorch/plugins/precision/half.py.

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

These checks are required after the changes to src/lightning/fabric/plugins/precision/deepspeed.py, src/lightning/fabric/plugins/precision/double.py, src/lightning/fabric/plugins/precision/fsdp.py, src/lightning/fabric/plugins/precision/half.py, src/lightning/fabric/plugins/precision/transformer_engine.py, src/lightning/pytorch/plugins/precision/deepspeed.py, src/lightning/pytorch/plugins/precision/double.py, src/lightning/pytorch/plugins/precision/fsdp.py, src/lightning/pytorch/plugins/precision/half.py.


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.

Copy link
Contributor

@awaelchli awaelchli left a comment

Choose a reason for hiding this comment

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

Should we add the corresponding unit-tests too?

@carmocca
Copy link
Contributor Author

carmocca commented Sep 7, 2023

@awaelchli Do you want a test for each class for both trainer and fabric?

@awaelchli
Copy link
Contributor

That's a possibility yes. Or a parameterized test for the class in a test_all.py file in the plugins folder if the assertion is the same for all.

@carmocca carmocca requested a review from Borda as a code owner September 7, 2023 16:39
@carmocca
Copy link
Contributor Author

carmocca commented Sep 7, 2023

Done

@mergify mergify bot added the has conflicts label Sep 7, 2023
@mergify mergify bot removed the has conflicts label Sep 7, 2023
Copy link
Member

@justusschock justusschock left a comment

Choose a reason for hiding this comment

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

looks good, just a small suggestion for future proofing :)

@mergify mergify bot added the ready PRs ready to be merged label Sep 14, 2023
@carmocca carmocca merged commit eb3b96d into master Sep 14, 2023
@carmocca carmocca deleted the carmocca/default-dtype-finally branch September 14, 2023 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fabric lightning.fabric.Fabric pl Generic label for PyTorch Lightning package plugin ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants