-
Notifications
You must be signed in to change notification settings - Fork 26k
Follow up on #161891 move additions to stable shim and use version guards #168025
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
Follow up on #161891 move additions to stable shim and use version guards #168025
Conversation
…ards [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/168025
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 4 PendingAs of commit 484f5cd with merge base 1c04a43 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Attention! PyTorch one of the C-stable API file was changedYou MUST NOT change existing function declarations in this, as this header defines a stable C ABI. If you need to change the signature for a function, introduce a new v2 version of the function and modify code generation to target the new version of the function. Caused by: |
… version guards" Address #161891 (comment) [ghstack-poisoned]
|
Starting merge as part of PR stack under #167962 |
This is tested by #167962 which ensures we get compilation errors when using functions that convert Device/HeaderOnlyArrayRef to StableIValue and target 2.9 Pull Request resolved: #167802 Approved by: https://github.com/janeyx99 ghstack dependencies: #168025
Tests are split into libtorch_agnostic_2_9_extension and libtorch_agnostic_2_10_extension depending on the minimum version they should compile+run in Pull Request resolved: #167803 Approved by: https://github.com/janeyx99 ghstack dependencies: #168025, #167802
Splits each torch library registration in the 2.10 folder into its own file -- I had a script that parsed kernel.cpp to do this but I felt like forcing this responsibility on the user might be less error prone Compiles each file targetting 2.9 and asserts that compilation fails. (There are 2 2.9 kernels we use as negative tests where compilation is expected to succeed) Pull Request resolved: #167962 Approved by: https://github.com/janeyx99 ghstack dependencies: #168025, #167802, #167803, #167804
Unclear which PR in the ghstack caused the ROCm failure. Stack was (oldest at bottom): - pytorch#167962 - pytorch#167804 - pytorch#167803 - pytorch#167802 - pytorch#168025
Unclear which PR in the ghstack caused the ROCm failure. Stack was (oldest at bottom): - #167962 - #167804 - #167803 - #167802 - #168025 Fixes the following test: PYTORCH_TEST_WITH_ROCM=1 python test/cpp_extensions/libtorch_agnostic_2_10_extension/test_version_compatibility.py FunctionVersionCompatibilityTest.test_mv_tensor_accessor_cuda_works_with_2_9 Pull Request resolved: #168087 Approved by: https://github.com/jeffdaily, https://github.com/janeyx99 Co-authored-by: Jeff Daily <jeff.daily@amd.com> Co-authored-by: Jane (Yuan) Xu <31798555+janeyx99@users.noreply.github.com>
~This PR does change the semantics of the >> operator by using STD_TORCH_CHECK to throw the error instead of TORCH_CHECK. Jane (who is writing this message) thinks it is okay because it is the error case when an invalid MemoryFormat or Layout is getting passed into >>, so the UX benefits of TORCH_CHECK over STD_TORCH_CHECK there are not significant enough to warrant making a new copy of Layout and MemoryFormat's >> APIs.~ Never mind! We shouldn't change TORCH_CHECK to STD_TORCH_CHECK for core usage ever, cuz the traceback info and c10::Error is very much desired!! So the solution is to not migrate the >>s. I pushed new commits to the stack to remove the >> code, but for reference, 8a30179 has all the code that I ended up deleting. Pull Request resolved: #168034 Approved by: https://github.com/janeyx99 ghstack dependencies: #168025, #167802, #167803, #167804, #167962 Co-authored-by: Jane Xu <janeyx@meta.com>
Address #161891 (comment)
Stack from ghstack (oldest at bottom):