Skip to content

Conversation

@mikaylagawarecki
Copy link
Contributor

@mikaylagawarecki mikaylagawarecki commented Nov 14, 2024

Same as #140739 but for DTensor (move safe globals for DTensor to torch.distributed.tensor.__init__ and update error message to let user know torch.distributed.tensor must be imported to load DTensor)

Stack from ghstack (oldest at bottom):

Differential Revision: D65961690

cc @H-Huang @awgu @kwen2501 @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @c-p-i-o

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 14, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/140740

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

As of commit 9e9bdbc with merge base b379a28 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

mikaylagawarecki added a commit that referenced this pull request Nov 14, 2024
ghstack-source-id: c356d4d
Pull Request resolved: #140740
@mikaylagawarecki
Copy link
Contributor Author

@mikaylagawarecki has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 14, 2024
The additions should not have been gated on `if hasattr(torch.distributed, "dtensor")` 

With pytorch built with USE_DISTRIBUTED=1
```
>>> import torch
>>> hasattr(torch.distributed, "dtensor")
False
```


Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

[ghstack-poisoned]
@pytorch-bot pytorch-bot bot added the oncall: distributed Add this issue/PR to distributed oncall triage queue label Nov 15, 2024
The additions should not have been gated on `if hasattr(torch.distributed, "dtensor")` 

With pytorch built with USE_DISTRIBUTED=1
```
>>> import torch
>>> hasattr(torch.distributed, "dtensor")
False
```


Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o

[ghstack-poisoned]
@mikaylagawarecki
Copy link
Contributor Author

@mikaylagawarecki has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Same as #140739 but for DTensor (move safe globals for DTensor to `torch.distributed.tensor.__init__` and update error message to let user know `torch.distributed.tensor` must be imported to load DTensor)


Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o

[ghstack-poisoned]
Same as #140739 but for DTensor (move safe globals for DTensor to `torch.distributed.tensor.__init__` and update error message to let user know `torch.distributed.tensor` must be imported to load DTensor)


Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o

[ghstack-poisoned]
@mikaylagawarecki mikaylagawarecki marked this pull request as ready for review November 15, 2024 20:20
Same as #140739 but for DTensor (move safe globals for DTensor to `torch.distributed.tensor.__init__` and update error message to let user know `torch.distributed.tensor` must be imported to load DTensor)


Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o

[ghstack-poisoned]
mikaylagawarecki added a commit that referenced this pull request Nov 15, 2024
ghstack-source-id: 90fb387
Pull Request resolved: #140740
Same as #140739 but for DTensor (move safe globals for DTensor to `torch.distributed.tensor.__init__` and update error message to let user know `torch.distributed.tensor` must be imported to load DTensor)


Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o

[ghstack-poisoned]
Same as #140739 but for DTensor (move safe globals for DTensor to `torch.distributed.tensor.__init__` and update error message to let user know `torch.distributed.tensor` must be imported to load DTensor)


Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o

[ghstack-poisoned]
Same as #140739 but for DTensor (move safe globals for DTensor to `torch.distributed.tensor.__init__` and update error message to let user know `torch.distributed.tensor` must be imported to load DTensor)


Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o

[ghstack-poisoned]
Same as #140739 but for DTensor (move safe globals for DTensor to `torch.distributed.tensor.__init__` and update error message to let user know `torch.distributed.tensor` must be imported to load DTensor)


Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o

[ghstack-poisoned]
@wz337
Copy link
Contributor

wz337 commented Nov 18, 2024

Thanks for helping to fix it!

pytorchmergebot pushed a commit that referenced this pull request Nov 22, 2024
…ually (#141300)

#140739 and #140740 made it such that `get_safe_globals` no longer return an empty List by default

This caused some tests that check the content of `get_safe_globals` to fail, in particular when run individually (they didn't fail in test suite as other tests ran before them called `clear_safe_globals`) but will fail when tests are run individually [T208186010](https://www.internalfb.com/intern/tasks/?t=208186010)

test_safe_globals_for_weights_only
test_safe_globals_context_manager_weights_only

This PR fixes that and also makes most tests calling `clear_safe_globals` use the `safe_globals` context manager rather than try: finally

Pull Request resolved: #141300
Approved by: https://github.com/awgu
Ryo-not-rio pushed a commit to Ryo-not-rio/pytorch that referenced this pull request Dec 2, 2024
Same as pytorch#140739 but for DTensor (move safe globals for DTensor to `torch.distributed.tensor.__init__` and update error message to let user know `torch.distributed.tensor` must be imported to load DTensor)

Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

Pull Request resolved: pytorch#140740
Approved by: https://github.com/malfet
ghstack dependencies: pytorch#140739
Ryo-not-rio pushed a commit to Ryo-not-rio/pytorch that referenced this pull request Dec 2, 2024
…ually (pytorch#141300)

pytorch#140739 and pytorch#140740 made it such that `get_safe_globals` no longer return an empty List by default

This caused some tests that check the content of `get_safe_globals` to fail, in particular when run individually (they didn't fail in test suite as other tests ran before them called `clear_safe_globals`) but will fail when tests are run individually [T208186010](https://www.internalfb.com/intern/tasks/?t=208186010)

test_safe_globals_for_weights_only
test_safe_globals_context_manager_weights_only

This PR fixes that and also makes most tests calling `clear_safe_globals` use the `safe_globals` context manager rather than try: finally

Pull Request resolved: pytorch#141300
Approved by: https://github.com/awgu
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
Same as pytorch#140739 but for DTensor (move safe globals for DTensor to `torch.distributed.tensor.__init__` and update error message to let user know `torch.distributed.tensor` must be imported to load DTensor)

Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

Pull Request resolved: pytorch#140740
Approved by: https://github.com/malfet
ghstack dependencies: pytorch#140739
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
…ually (pytorch#141300)

pytorch#140739 and pytorch#140740 made it such that `get_safe_globals` no longer return an empty List by default

This caused some tests that check the content of `get_safe_globals` to fail, in particular when run individually (they didn't fail in test suite as other tests ran before them called `clear_safe_globals`) but will fail when tests are run individually [T208186010](https://www.internalfb.com/intern/tasks/?t=208186010)

test_safe_globals_for_weights_only
test_safe_globals_context_manager_weights_only

This PR fixes that and also makes most tests calling `clear_safe_globals` use the `safe_globals` context manager rather than try: finally

Pull Request resolved: pytorch#141300
Approved by: https://github.com/awgu
fmo-mt pushed a commit to fmo-mt/pytorch that referenced this pull request Dec 11, 2024
Same as pytorch#140739 but for DTensor (move safe globals for DTensor to `torch.distributed.tensor.__init__` and update error message to let user know `torch.distributed.tensor` must be imported to load DTensor)

Differential Revision: [D65961690](https://our.internmc.facebook.com/intern/diff/D65961690)

Pull Request resolved: pytorch#140740
Approved by: https://github.com/malfet
ghstack dependencies: pytorch#140739
@github-actions github-actions bot deleted the gh/mikaylagawarecki/289/head branch December 20, 2024 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged oncall: distributed Add this issue/PR to distributed oncall triage queue topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants