Skip to content

Conversation

@FFFrog
Copy link
Collaborator

@FFFrog FFFrog commented Apr 30, 2025

Stack from ghstack (oldest at bottom):

torch/csrc/utils.h should be device-independent. Currently, it contains CUDA-related implementations, which indirectly causes the failure of ROCm testing (The reason is that the ROCm test environment shouldn`t expose HIP-related header files, which causes the JIT compilation to fail during testing)

Therefore, move CUDA-related implementations to torch/csrc/cuda/utils.h.

Question:
This change may introduce BC-breack.
I searched for this function globally on github and I think the impact is very small.

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Apr 30, 2025

🔗 Helpful Links

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

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:

❌ 1 New Failure, 1 Unrelated Failure

As of commit 2d843b5 with merge base 59a8aa1 (image):

NEW FAILURE - The following job has failed:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

@FFFrog
Copy link
Collaborator Author

FFFrog commented Apr 30, 2025

@pytorchbot label "topic: not user facing"

[ghstack-poisoned]
@FFFrog FFFrog requested review from eqy and syed-ahmed as code owners April 30, 2025 07:38
@cyyever cyyever added the ciflow/rocm Trigger "default" config CI on ROCm label Apr 30, 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.

There is a major difference between "no breakage" and "the impact is very small". We should be careful to aim for "no breakage" in general. And treat everything else as BC-breaking.
From the github search, I see no use outside of pytorch codebase or clones. So sounds good!

@FFFrog
Copy link
Collaborator Author

FFFrog commented May 4, 2025

There is a major difference between "no breakage" and "the impact is very small". We should be careful to aim for "no breakage" in general. And treat everything else as BC-breaking. From the github search, I see no use outside of pytorch codebase or clones. So sounds good!

Thank you a lot.

@FFFrog
Copy link
Collaborator Author

FFFrog commented May 4, 2025

@pytorchbot merge -i

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label May 4, 2025
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 1 checks: rocm / linux-focal-rocm-py3.10 / test (default, 2, 6, linux.rocm.gpu.2)

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
Copy link
Collaborator

Starting merge as part of PR stack under #152522

pytorchmergebot pushed a commit that referenced this pull request May 4, 2025
As the title stated.

**Question:**

I have carefully looked through all the .h files in Tensor.cpp and from my perspective this file does not make sense. Does anyone know what the background is for doing this?

Pull Request resolved: #152522
Approved by: https://github.com/Skylion007, https://github.com/albanD, https://github.com/eqy
ghstack dependencies: #152512, #152513, #152521
@github-actions github-actions bot deleted the gh/fffrog/90/head branch June 15, 2025 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/rocm Trigger "default" config CI on ROCm ciflow/trunk Trigger trunk jobs on your pull request Merged open source topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants