[FSDP2] Make module-to-state mapping use weakrefs #139650

awgu · 2024-11-04T18:59:50Z

Stack from ghstack (oldest at bottom):

-> [FSDP2] Make module-to-state mapping use weakrefs #139650

Without this, del model does not free memory of a module with FSDP2 applied.

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

[ghstack-poisoned]

pytorch-bot · 2024-11-04T18:59:54Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

❌ 10 New Failures, 1 Cancelled Job, 4 Unrelated Failures

As of commit ef985b5 with merge base 0e4d426 ():

NEW FAILURES - The following jobs have failed:

linux-binary-libtorch-cxx11-abi / libtorch-cpu-shared-with-deps-cxx11-abi-build / build (gh)
bash: /builder/libtorch/build.sh: No such file or directory
linux-binary-libtorch-pre-cxx11 / libtorch-cpu-shared-with-deps-pre-cxx11-build / build (gh)
bash: /builder/libtorch/build.sh: No such file or directory
linux-binary-manywheel / manywheel-py3_9-cuda11_8-build / build (gh)
bash: /builder/manywheel/build.sh: No such file or directory
linux-binary-manywheel / manywheel-py3_9-cuda12_1-build / build (gh)
bash: /builder/manywheel/build.sh: No such file or directory
linux-binary-manywheel / manywheel-py3_9-cuda12_4-build / build (gh)
bash: /builder/manywheel/build.sh: No such file or directory
linux-binary-manywheel-split / manywheel-py3_9-cuda11_8-build / build (gh)
bash: /builder/manywheel/build.sh: No such file or directory
linux-binary-manywheel-split / manywheel-py3_9-cuda12_1-build / build (gh)
bash: /builder/manywheel/build.sh: No such file or directory
linux-binary-manywheel-split / manywheel-py3_9-cuda12_4-build / build (gh)
bash: /builder/manywheel/build.sh: No such file or directory
pull / linux-jammy-py3-clang12-executorch / test (executorch, 1, 1, linux.2xlarge) (gh)
ERROR: No matching distribution found for torch==2.5.0.dev20240901
trunk / linux-focal-cuda12.4-py3.10-gcc9-sm86 / test (default, 5, 5, linux.g5.4xlarge.nvidia.gpu) (gh)
'test/inductor/test_cudacodecache.py::TestCUDACodeCache::test_async_compile'

CANCELLED JOB - The following job was cancelled. Please retry:

periodic / linux-focal-cuda11.8-py3.9-gcc9-experimental-split-build / build (gh)
##[error]The operation was canceled.

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

pull / linux-focal-py3.11-clang10 / test (default, 1, 4, linux.4xlarge) (gh) (disabled by #136125, #137026, #137027)
inductor/test_extension_backend.py::ExtensionBackendTests::test_open_device_registration
pull / linux-focal-py3.12-clang10 / test (default, 1, 4, linux.4xlarge) (gh) (disabled by #136125, #137026, #137027)
inductor/test_extension_backend.py::ExtensionBackendTests::test_open_device_registration
pull / linux-focal-py3.9-clang10 / test (default, 1, 4, linux.4xlarge) (gh) (disabled by #136125, #137026, #137027)
inductor/test_extension_backend.py::ExtensionBackendTests::test_open_device_registration
pull / linux-jammy-py3.9-gcc11 / test (default, 1, 4, linux.2xlarge) (gh) (disabled by #136125, #137026, #137027)
inductor/test_extension_backend.py::ExtensionBackendTests::test_open_device_registration

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

ghstack-source-id: 8205146 Pull Request resolved: #139650

yf225

thanks! (I also have run into this before)

awgu · 2024-11-04T22:26:55Z

@pytorchbot merge -i

pytorchmergebot · 2024-11-04T22:28:43Z

Merge started

Your change will be merged while ignoring the following 8 checks: pull / linux-focal-py3.9-clang10 / test (default, 1, 4, linux.4xlarge), pull / linux-focal-py3.11-clang10 / test (default, 1, 4, linux.4xlarge), pull / linux-focal-py3.12-clang10 / test (default, 1, 4, linux.4xlarge), pull / linux-jammy-py3.9-gcc11 / test (default, 1, 4, linux.2xlarge), pull / linux-jammy-py3-clang12-executorch / test (executorch, 1, 1, linux.2xlarge), linux-binary-manywheel-split / manywheel-py3_9-cuda12_4-build / build, linux-binary-manywheel-split / manywheel-py3_9-cuda11_8-build / build, linux-binary-manywheel-split / manywheel-py3_9-cuda12_1-build / build

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2024-11-04T22:39:46Z

Merge failed

Reason: 2 jobs have failed, first few of them are: linux-binary-manywheel / manywheel-py3_9-cuda12_1-build / build, linux-binary-manywheel / manywheel-py3_9-cuda11_8-build / build

Details for Dev Infra team

Raised by workflow job

Skylion007 · 2024-11-05T00:28:23Z

Seems like a good backport candidate?

awgu · 2024-11-05T02:15:03Z

@pytorchbot merge -f "unrelated failures"

pytorchmergebot · 2024-11-05T02:16:38Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

awgu · 2024-11-05T03:39:32Z

Seems like a good backport candidate?

Yes, possibly. We should see if any internal issues arise from this, but I suspect not.

Pull Request resolved: #139759 Approved by: https://github.com/weifengpy, https://github.com/wz337 ghstack dependencies: #139650

Pull Request resolved: pytorch#139759 Approved by: https://github.com/weifengpy, https://github.com/wz337 ghstack dependencies: pytorch#139650

[FSDP2] Make module-to-state mapping use weakrefs

ef985b5

[ghstack-poisoned]

pytorch-bot bot added oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: distributed (fsdp) release notes category labels Nov 4, 2024

awgu pushed a commit that referenced this pull request Nov 4, 2024

[FSDP2] Make module-to-state mapping use weakrefs

25df33c

ghstack-source-id: 8205146 Pull Request resolved: #139650

awgu added release notes: distributed (fsdp2) release notes category ciflow/periodic Trigger jobs ran periodically on master (periodic.yml) on the PR ciflow/inductor and removed release notes: distributed (fsdp) release notes category labels Nov 4, 2024

awgu mentioned this pull request Nov 4, 2024

[RFC] Per-Parameter-Sharding FSDP #114299

Open

yf225 approved these changes Nov 4, 2024

View reviewed changes

awgu marked this pull request as ready for review November 4, 2024 22:11

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 4, 2024

pytorchmergebot added the merging label Nov 4, 2024

pytorchmergebot removed the merging label Nov 4, 2024

pytorchmergebot added the merging label Nov 5, 2024

pytorchmergebot closed this in 9039fbb Nov 5, 2024

pytorchmergebot added Merged and removed merging labels Nov 5, 2024

awgu mentioned this pull request Nov 5, 2024

Add current FSDP2 path to old composable FSDP1 warning #139759

Closed

pytorchmergebot pushed a commit that referenced this pull request Nov 6, 2024

Add current FSDP2 path to old composable FSDP1 warning (#139759)

39ede99

Pull Request resolved: #139759 Approved by: https://github.com/weifengpy, https://github.com/wz337 ghstack dependencies: #139650

github-actions bot deleted the gh/awgu/655/head branch December 6, 2024 02:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[FSDP2] Make module-to-state mapping use weakrefs #139650

[FSDP2] Make module-to-state mapping use weakrefs #139650

Uh oh!

awgu commented Nov 4, 2024 •

edited

Loading

Uh oh!

pytorch-bot bot commented Nov 4, 2024 •

edited

Loading

Uh oh!

yf225 left a comment

Uh oh!

awgu commented Nov 4, 2024

Uh oh!

pytorchmergebot commented Nov 4, 2024

Uh oh!

pytorchmergebot commented Nov 4, 2024

Uh oh!

Skylion007 commented Nov 5, 2024

Uh oh!

awgu commented Nov 5, 2024

Uh oh!

pytorchmergebot commented Nov 5, 2024

Uh oh!

awgu commented Nov 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[FSDP2] Make module-to-state mapping use weakrefs #139650

[FSDP2] Make module-to-state mapping use weakrefs #139650

Uh oh!

Conversation

awgu commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

❌ 10 New Failures, 1 Cancelled Job, 4 Unrelated Failures

Uh oh!

yf225 left a comment

Choose a reason for hiding this comment

Uh oh!

awgu commented Nov 4, 2024

Uh oh!

pytorchmergebot commented Nov 4, 2024

Merge started

Uh oh!

pytorchmergebot commented Nov 4, 2024

Merge failed

Uh oh!

Skylion007 commented Nov 5, 2024

Uh oh!

awgu commented Nov 5, 2024

Uh oh!

pytorchmergebot commented Nov 5, 2024

Merge started

Uh oh!

awgu commented Nov 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

awgu commented Nov 4, 2024 •

edited

Loading

pytorch-bot bot commented Nov 4, 2024 •

edited

Loading