Skip to content

Conversation

awgu
Copy link
Collaborator

@awgu awgu commented Dec 14, 2022

Stack from ghstack:

This PR adds FSDP and composable API files to .lintrunner.toml so that (1) lintrunner enforces that those files are formatted and (2) lintrunner f formats those files for you.

There are two requirements here (see https://github.com/pytorch/pytorch/wiki/lintrunner for details):

  1. Install lintrunner:
pip install lintrunner
lintrunner init
  1. lintrunner f before you finalize your PR, which would now be enforced by CI after this PR.

The code changes in this PR outside of .lintrunner.toml are the result of lintrunner f.


I only plan to land this PR if all of the composable API developers agree that this is something that makes sense and is not too intrusive to the workflow.

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Dec 14, 2022
@pytorch-bot
Copy link

pytorch-bot bot commented Dec 14, 2022

🔗 Helpful Links

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

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

✅ No Failures

As of commit 4d1e5a2:
💚 Looks good so far! There are no failures yet. 💚

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

I am trying to see if we can get ufmt to cover FSDP and composable API files so that we do not need to keep having separate PRs to ufmt the files.

[ghstack-poisoned]
This PR adds FSDP and composable API files to `.lintrunner.toml` so that (1) lintrunner enforces that those files are formatted and (2) `lintrunner f` formats those files for you.

This should not impede developer productivity because lintrunner comes with PyTorch. The only added step in the development process is the `lintrunner f`, which is enforced on PRs.

[ghstack-poisoned]
This PR adds FSDP and composable API files to `.lintrunner.toml` so that (1) lintrunner enforces that those files are formatted and (2) `lintrunner f` formats those files for you.

There are two requirements here (see https://github.com/pytorch/pytorch/wiki/lintrunner for details):
1. Install lintrunner: 
```
pip install lintrunner
lintrunner init
```
2. `lintrunner f` before you finalize your PR, which is now enforced by CI.

The code changes outside of `.lintrunner.toml` are exactly the result of `lintrunner f`.

[ghstack-poisoned]
awgu pushed a commit that referenced this pull request Dec 15, 2022
ghstack-source-id: 423fad2
Pull Request resolved: #90873
@awgu awgu marked this pull request as ready for review December 15, 2022 09:38
This PR adds FSDP and composable API files to `.lintrunner.toml` so that (1) lintrunner enforces that those files are formatted and (2) `lintrunner f` formats those files for you.

There are two requirements here (see https://github.com/pytorch/pytorch/wiki/lintrunner for details):
1. Install lintrunner: 
```
pip install lintrunner
lintrunner init
```
2. `lintrunner f` before you finalize your PR, which would now be enforced by CI after this PR.

The code changes in this PR outside of `.lintrunner.toml` are the result of `lintrunner f`.

---

I only plan to land this PR if all of the composable API developers agree that this is something that makes sense and is not too intrusive to the workflow.


[ghstack-poisoned]
This PR adds FSDP and composable API files to `.lintrunner.toml` so that (1) lintrunner enforces that those files are formatted and (2) `lintrunner f` formats those files for you.

There are two requirements here (see https://github.com/pytorch/pytorch/wiki/lintrunner for details):
1. Install lintrunner: 
```
pip install lintrunner
lintrunner init
```
2. `lintrunner f` before you finalize your PR, which would now be enforced by CI after this PR.

The code changes in this PR outside of `.lintrunner.toml` are the result of `lintrunner f`.

---

I only plan to land this PR if all of the composable API developers agree that this is something that makes sense and is not too intrusive to the workflow.


[ghstack-poisoned]
awgu pushed a commit that referenced this pull request Jan 17, 2023
ghstack-source-id: 07be631
Pull Request resolved: #90873
@awgu awgu added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 17, 2023
Copy link
Contributor

@mrshenli mrshenli left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @awgu

Copy link
Contributor

@rohan-varma rohan-varma left a comment

Choose a reason for hiding this comment

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

Great step in the right direction of enforcing lint!

@awgu
Copy link
Collaborator Author

awgu commented Jan 17, 2023

Failure are related to Dynamo and should be unrelated.

2023-01-17T17:30:37.5734804Z Traceback (most recent call last):
2023-01-17T17:30:37.5735042Z   File "/opt/conda/lib/python3.10/site-packages/torch/_dynamo/convert_frame.py", line 440, in _compile
2023-01-17T17:30:37.5735462Z ##[endgroup]
2023-01-17T17:30:37.5735569Z     check_fn = CheckFunctionManager(
2023-01-17T17:30:37.5735804Z   File "/opt/conda/lib/python3.10/site-packages/torch/_dynamo/guards.py", line 526, in __init__
2023-01-17T17:30:37.5735903Z     local_builder = GuardBuilder(
2023-01-17T17:30:37.5736137Z   File "/opt/conda/lib/python3.10/site-packages/torch/_dynamo/guards.py", line 109, in __init__
2023-01-17T17:30:37.5736278Z     self.scope["__builtins__"].__dict__[name] = package_module  # type: ignore[index]
2023-01-17T17:30:37.5736452Z AttributeError: 'dict' object has no attribute '__dict__'

Edit: Let me rebase to be safe.

awgu pushed a commit to awgu/pytorch that referenced this pull request Jan 17, 2023
This PR adds FSDP and composable API files to `.lintrunner.toml` so that (1) lintrunner enforces that those files are formatted and (2) `lintrunner f` formats those files for you.

There are two requirements here (see https://github.com/pytorch/pytorch/wiki/lintrunner for details):
1. Install lintrunner: 
```
pip install lintrunner
lintrunner init
```
2. `lintrunner f` before you finalize your PR, which would now be enforced by CI after this PR.

The code changes in this PR outside of `.lintrunner.toml` are the result of `lintrunner f`.

---

I only plan to land this PR if all of the composable API developers agree that this is something that makes sense and is not too intrusive to the workflow.


[ghstack-poisoned]
This PR adds FSDP and composable API files to `.lintrunner.toml` so that (1) lintrunner enforces that those files are formatted and (2) `lintrunner f` formats those files for you.

There are two requirements here (see https://github.com/pytorch/pytorch/wiki/lintrunner for details):
1. Install lintrunner: 
```
pip install lintrunner
lintrunner init
```
2. `lintrunner f` before you finalize your PR, which would now be enforced by CI after this PR.

The code changes in this PR outside of `.lintrunner.toml` are the result of `lintrunner f`.

---

I only plan to land this PR if all of the composable API developers agree that this is something that makes sense and is not too intrusive to the workflow.


[ghstack-poisoned]
awgu pushed a commit that referenced this pull request Jan 18, 2023
ghstack-source-id: abeffc2
Pull Request resolved: #90873
@awgu
Copy link
Collaborator Author

awgu commented Jan 18, 2023

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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

yhcharles added a commit that referenced this pull request Mar 11, 2023
…istributed.py`"



As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.


[ghstack-poisoned]
yhcharles added a commit that referenced this pull request Mar 13, 2023
…istributed.py`"



As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.


[ghstack-poisoned]
yhcharles added a commit that referenced this pull request Mar 13, 2023
…istributed.py`"



As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.


[ghstack-poisoned]
yhcharles added a commit that referenced this pull request Mar 13, 2023
…istributed.py`"



As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.


[ghstack-poisoned]
yhcharles added a commit that referenced this pull request Mar 17, 2023
…etup ufmt for `distributed.py`"



As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.


[ghstack-poisoned]
yhcharles added a commit that referenced this pull request Mar 17, 2023
…istributed.py`"



As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.


[ghstack-poisoned]
yhcharles added a commit that referenced this pull request Mar 17, 2023
…etup ufmt for `distributed.py`"



As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.


[ghstack-poisoned]
yhcharles added a commit that referenced this pull request Mar 17, 2023
…istributed.py`"



As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.


[ghstack-poisoned]
pytorchmergebot pushed a commit that referenced this pull request Mar 17, 2023
…py` (#96597)

As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.

Pull Request resolved: #96597
Approved by: https://github.com/rohan-varma
yhcharles added a commit that referenced this pull request Mar 22, 2023
…etup ufmt for `distributed.py`"



As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.


[ghstack-poisoned]
yhcharles added a commit that referenced this pull request Mar 22, 2023
…istributed.py`"



As we already enabled ufmt for composable APIs in #90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.


[ghstack-poisoned]
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Mar 23, 2023
…py` (#96597)

As we already enabled ufmt for composable APIs in pytorch/pytorch#90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.

Pull Request resolved: pytorch/pytorch#96597
Approved by: https://github.com/rohan-varma
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Mar 27, 2023
…py` (#96597)

As we already enabled ufmt for composable APIs in pytorch/pytorch#90873, it seems a good idea to enable ufmt for other distributed APIs as well. This change setup ufmt for DDP.

Pull Request resolved: pytorch/pytorch#96597
Approved by: https://github.com/rohan-varma
@facebook-github-bot facebook-github-bot deleted the gh/awgu/278/head branch June 8, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants