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

migrates prototype/mixed_precision to configs #1854

Merged
merged 24 commits into from
Mar 8, 2025
Merged

migrates prototype/mixed_precision to configs #1854

merged 24 commits into from
Mar 8, 2025

Conversation

vkuzo
Copy link
Contributor

@vkuzo vkuzo commented Mar 7, 2025

Summary:

Note: had to remove int4/int8 functionality to simplify the refactor.
Whoever uses this script next and needs that functionality can add this
back.

Test Plan:

pytest test/prototype/test_mixed_precision.py -s -x

Reviewers:

Subscribers:

Tasks:

Tags:

vkuzo added 4 commits March 7, 2025 06:48
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
Copy link

pytorch-bot bot commented Mar 7, 2025

🔗 Helpful Links

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

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

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

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 7, 2025
vkuzo added a commit that referenced this pull request Mar 7, 2025
Summary:

Note: had to remove int4/int8 functionality to simplify the refactor.
Whoever uses this script next and needs that functionality can add this
back.

Test Plan:

```
pytest test/prototype/test_mixed_precision.py -s -x
```

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 2b0f46d458e69e6c16aefe4a8644c44094b584cb
ghstack-comment-id: 2706815985
Pull Request resolved: #1854
@vkuzo vkuzo added the topic: not user facing Use this tag if you don't want this PR to show up in release notes label Mar 7, 2025
vkuzo added 2 commits March 7, 2025 09:51
[ghstack-poisoned]
[ghstack-poisoned]
vkuzo added 8 commits March 7, 2025 11:39
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]


# for bc
intN_weight_only = IntNWeightOnlyConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

Imo these prototype folders we should just break BC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but it's actually easier to keep BC here (to not change callsites) - if someone wants to change the callsites of all these prototype features, that would sgtm in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess, but also changing callsites within a repro is - I find - not that hard and ultimately helps create a more unified api as we move everything to configs + registrations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@drisspg , I take it back. See how the scope of #1851 expanded from removing the bc name. Some of these other prototypes have even more callsites throughout the codebase, some of those callsites aren't easily testable, etc - I just don't think it's worth the effort to tie removing old names to this PR stack since we have to keep BC anyways for some of the APIs.

Copy link
Contributor

@drisspg drisspg left a comment

Choose a reason for hiding this comment

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

See comment

[ghstack-poisoned]
vkuzo added 9 commits March 8, 2025 06:15
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@vkuzo vkuzo changed the base branch from gh/vkuzo/57/head to main March 8, 2025 20:18
@vkuzo vkuzo merged commit bc4f51d into main Mar 8, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. topic: not user facing Use this tag if you don't want this PR to show up in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants