KEMBAR78
Move Float8 variations to headeronly by janeyx99 · Pull Request #159415 · pytorch/pytorch · GitHub
Skip to content

Conversation

@janeyx99
Copy link
Contributor

@janeyx99 janeyx99 commented Jul 29, 2025

This PR is a big copy pasta from c10/util/Float8* -> torch/headeronly/util/ which is why we are breaking PR sanity :C (sorry @albanD!).

Why is it not a clean copy paste?

  • For BC reasons, we have to keep the old c10 file around so that OSS devs relying on those files can still get the same APIs
  • Because we reexpose APIs that are headeronly through torch::headeronly, so there is an extra chunk of code in the new torch::headeronly files to do that.

Outside of the copy paste, I:

  • changed the tests to call torch::headeronly instead of c10
  • updated header_only_apis.txt
  • added // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions) to pass lint (which was previously skipped for -inl.h files)

Stack from ghstack (oldest at bottom):

cc @albanD

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 29, 2025

🔗 Helpful Links

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

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

❌ 11 Cancelled Jobs, 1 Unrelated Failure

As of commit b3e35a0 with merge base d2e0258 (image):

CANCELLED JOBS - The following jobs were cancelled. Please retry:

UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:

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

janeyx99 added a commit that referenced this pull request Jul 29, 2025
ghstack-source-id: 27c611f
Pull Request resolved: #159415
This PR is a big copy pasta from `c10/util/Float8*` -> `torch/headeronly/util/` which is why we are breaking PR sanity :C (sorry albanD!).

Why is it not a clean copy paste?
- For BC reasons, we have to keep the old c10 file around so that OSS devs relying on those files can still get the same APIs
- Because we reexpose APIs that are headeronly through torch::headeronly, so there is an extra chunk of code in the new torch::headeronly files to do that.

Outside of the copy paste, I changed the tests to call torch::headeronly instead of c10 and updated header_only_apis.txt.




cc albanD

[ghstack-poisoned]
@janeyx99 janeyx99 requested review from albanD and desertfire July 31, 2025 15:15
@janeyx99 janeyx99 added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 31, 2025
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

SGTM

This PR is a big copy pasta from `c10/util/Float8*` -> `torch/headeronly/util/` which is why we are breaking PR sanity :C (sorry albanD!).

Why is it not a clean copy paste?
- For BC reasons, we have to keep the old c10 file around so that OSS devs relying on those files can still get the same APIs
- Because we reexpose APIs that are headeronly through torch::headeronly, so there is an extra chunk of code in the new torch::headeronly files to do that.

Outside of the copy paste, I:
- changed the tests to call torch::headeronly instead of c10
- updated header_only_apis.txt
- added `// NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)` to pass lint (which was previously skipped for -inl.h files)




cc albanD

[ghstack-poisoned]
This PR is a big copy pasta from `c10/util/Float8*` -> `torch/headeronly/util/` which is why we are breaking PR sanity :C (sorry albanD!).

Why is it not a clean copy paste?
- For BC reasons, we have to keep the old c10 file around so that OSS devs relying on those files can still get the same APIs
- Because we reexpose APIs that are headeronly through torch::headeronly, so there is an extra chunk of code in the new torch::headeronly files to do that.

Outside of the copy paste, I:
- changed the tests to call torch::headeronly instead of c10
- updated header_only_apis.txt
- added `// NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)` to pass lint (which was previously skipped for -inl.h files)




cc albanD

[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Starting merge as part of PR stack under #159411

pytorchmergebot pushed a commit that referenced this pull request Jul 31, 2025
Pull Request resolved: #159411
Approved by: https://github.com/albanD
ghstack dependencies: #159415
yangw-dev pushed a commit that referenced this pull request Aug 1, 2025
This PR is a big copy pasta from `c10/util/Float8*` -> `torch/headeronly/util/` which is why we are breaking PR sanity :C (sorry @albanD!).

Why is it not a clean copy paste?
- For BC reasons, we have to keep the old c10 file around so that OSS devs relying on those files can still get the same APIs
- Because we reexpose APIs that are headeronly through torch::headeronly, so there is an extra chunk of code in the new torch::headeronly files to do that.

Outside of the copy paste, I:
- changed the tests to call torch::headeronly instead of c10
- updated header_only_apis.txt
- added `// NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)` to pass lint (which was previously skipped for -inl.h files)

Pull Request resolved: #159415
Approved by: https://github.com/albanD
yangw-dev pushed a commit that referenced this pull request Aug 1, 2025
Pull Request resolved: #159411
Approved by: https://github.com/albanD
ghstack dependencies: #159415
pytorchmergebot pushed a commit that referenced this pull request Aug 1, 2025
Pull Request resolved: #159416
Approved by: https://github.com/albanD
ghstack dependencies: #159415, #159411
@github-actions github-actions bot deleted the gh/janeyx99/290/head branch August 31, 2025 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged release notes: build release notes category skip-pr-sanity-checks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants