## Linked issue or supporting maintainer
Part of #201370
Supported by @ezyang
## BC-breaking?
No
## Summary (human written only)
<!-- TODO(ezyang): fill in -->
## Agent details (if applicable)
> Several indexing tests about bool / mask indexing build their masks (or
> 0-dim true/false indices) as uint8, which is deprecated (#201370). Switch
> them to bool so they keep testing what they are about once uint8 indexing
> becomes an error. Tests that are specifically about uint8 masks are left for
> that change.
>
> Test plan: test_indexing.py. The C++ test_api and MPS edits were not run
> locally.
>
> Authored with an AI assistant.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/201373
Approved by: https://github.com/Skylion007
`RNNTest.BidirectionalLSTMReverseForward_CUDA` in `cpp/test_api` fails consistently on ROCm MI210 (`linux-jammy-rocm-py3.11-mi200`, original run plus 2 reruns), e.g. https://github.com/pytorch/pytorch/actions/runs/37407982276/job/112113883767:
```
test/cpp/api/rnn.cpp:544: Failure
Expected equality of these values:
std::get<0>(std::get<1>(bi_output))[1][0][0].item<float>()
Which is: -0.240088522
std::get<0>(std::get<1>(reverse_output))[0][0][0].item<float>()
Which is: -0.240088537
```
**Root cause:** the ROCm 6.3 upgrade (#142152) switched the per-timestep output comparison in this test to `ASSERT_NEAR` with `tolerance = 1e-5`, but left the final hidden/cell state (`h_n`/`c_n`) checks as exact `ASSERT_EQ`. Those checks never actually ran on ROCm because `test_api` collected 0 tests there until #199440 ("Fix C++ tests silently not running on ROCm CI") landed. Now that it runs, `h_n` differs by ~1.5e-8 (float32 rounding). The reverse-direction `h_n` is the same value as the last reverse output the loop already compares with tolerance, so the exact check is inconsistent within the test itself.
**Fix:** use `ASSERT_NEAR` with the test's existing `tolerance` for the `h_n` and `c_n` checks. No new tolerance is introduced; CPU and CUDA behavior is unaffected beyond allowing the same 1e-5 slack already used for outputs.
**Testing:** the observed difference (1.5e-8) is well within the existing 1e-5 tolerance; relying on ROCm CI for the C++ test run.
This PR was authored with the help of an AI assistant.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/199922
Approved by: https://github.com/jeffdaily
Fixes#106700.
`torch::nn::functional::multi_head_attention_forward` divides before it validates:
```cpp
const auto head_dim = embed_dim / num_heads;
TORCH_CHECK(head_dim * num_heads == embed_dim, "embed_dim must be divisible by num_heads");
```
so `num_heads == 0` is an integer division by zero. That is not a catchable exception — the
process dies. Running the call in a subprocess on Windows exits with `0xC0000094`
(`STATUS_INTEGER_DIVIDE_BY_ZERO`) with no stdout, no stderr and no traceback; the original report
shows the same as SIGFPE on Linux.
### Half of this issue is already fixed — this is the other half
#186376 added `TORCH_CHECK(options.num_heads() > 0, ...)` to `MultiheadAttentionImpl::reset`
(`activation.cpp:504`) and landed as `e71ac681afa`. That closed the case in the issue *body*: the
module constructor. The reporter raised a second case in a follow-up comment — the functional
entry point — and it was never covered. This adds the same check with the same message,
immediately before the division.
@malfet approved the earlier #106778, which fixed both halves; it went stale on unrelated CI in
2023 and closed without landing.
### Test
`ModulesTest.MultiheadAttentionZeroNumHeads` covers both halves — the module constructor and the
functional call. #186376 already covered the constructor with `MultiheadAttentionRejectsZeroNumHeads`;
this adds the missing coverage for the functional entry point.
The crash cannot be caught in-process, so I verified through a `cpp_extension` harness run in a
subprocess. Against current headers:
```
stdout: (empty)
stderr: (empty)
exit code: 3221225620 # 0xC0000094, STATUS_INTEGER_DIVIDE_BY_ZERO
```
With this change, the same harness:
```
stdout: RAISED RuntimeError num_heads must be greater than 0
exit code: 0
```
`cl.exe /Zs test\cpp\api\modules.cpp` is clean against the build's flags; the gtest binary itself
is left to CI, as this machine cannot launch freshly linked executables.
Developed with AI assistance (Claude); I reproduced the crash, wrote the fix and ran the checks
above.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/195381
Approved by: https://github.com/drisspg
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
## Author Notes
Pretty mechanical, nothing special here.
## Agent Notes
Written by Claude Code:
> The two test directories are 48 findings, and they come apart cleanly: 43 are
> stubs and error checks that convert mechanically, and 5 are throws whose type is
> the point of the test they sit in. Getting those 5 wrong would leave the suite
> green while testing the wrong thing, which is the only real risk in this PR.
>
> The 43: `rng_extension.cpp` and `cpp_c10d_extension.cpp` are "not implemented" /
> "does not support" overrides (8 and 10); `aoti_inference/test.cpp` is CUDA API
> error checks (16); the rest are `aoti_custom_class.cpp`,
> `test_custom_class_registrations.cpp` (6, a toy stack machine's error paths) and
> the two `my_from_blob_*` extensions. None of their messages is asserted on
> anywhere - I grepped the tree for each, including the Python suites.
>
> The two `my_from_blob_*` sites get STD_TORCH_CHECK rather than TORCH_CHECK.
> Those TUs include only `torch/csrc/stable/*`, which reaches
> torch/headeronly/util/Exception.h but never c10/util/Exception.h, so TORCH_CHECK
> is not declared there at all. STD_TORCH_CHECK preserves the exception type
> exactly - it throws std::runtime_error, same as the raw throw - but not the
> message: like the serialization-codegen PR earlier in this stack, it prepends
> __func__, __FILE__ and __LINE__, so
>
> before: cudaMalloc failed
> after: my_from_blob_with_cuda_deleter, <path>/my_from_blob_with_deleter.cpp:73,
> cudaMalloc failed
>
> These are the only two sites in this PR where anything observable moves. Nothing
> asserts on either string, both are behind #ifdef LAE_USE_CUDA in a test
> extension, and STD_TORCH_CHECK is already the house style in the sibling
> libtorch_agn_2_* extensions (17 uses), so their messages are decorated the same
> way. The alternative was to leave these two raw and annotated; taking the
> decoration keeps the file consistent with its neighbours.
>
> The 5 that stay raw, with the reason on each site:
>
> ProcessGroupNCCLErrorsTest.cpp TestHeartbeatMonitor::runLoop, 16 lines
> above, catches std::runtime_error by name
> to set hasMonitorThreadCaughtError_. A
> c10::Error is not caught by that, and the
> throw happens on the monitor thread.
> dataloader.cpp The test rethrows e.original_exception and
> asserts it is still a std::invalid_argument
> - the point being that WorkerException
> round-trips the original type.
> parallel.cpp ParallelApplyRethrowsException is about a
> user exception escaping forward().
> FileStoreTest.cpp std::system_error carrying errno as a
> std::error_code.
> OpenRegException.cpp Reports a caller-supplied SourceLocation;
> a TORCH_CHECK there would stamp this
> file's own location instead.
>
> Two of those five are load-bearing in a way I could measure rather than argue,
> and the test plan shows the failures. `parallel.cpp` is the one that is not:
> its assertion is `ASSERT_THROWS_WITH(..., "Badness!")`, a substring match that
> would still pass after conversion. It is annotated for intent, not because it
> would break, and it seemed worth being explicit about that difference rather
> than implying all five were equally forced.
>
> The exclude list goes from 11 entries to 9.
>
> Test Plan:
>
> Half of this is CUDA or distributed and does not build here, so the two CUDA
> files - parallel.cpp and ProcessGroupNCCLErrorsTest.cpp - deliberately received
> comment-only edits, which is the safest possible change to code a configuration
> cannot compile.
>
> For the rest, what matters is whether the *changed lines* were compiled, not the
> files: three of them sit behind preprocessor guards that a plain CPU build skips
> straight over. `aoti_inference/test.cpp`'s 16 conversions are inside
> `#if defined(USE_CUDA) || defined(USE_ROCM)`, and the two `my_from_blob_*` sites
> are inside `#ifdef LAE_USE_CUDA`, which setup.py only defines when CUDA is
> available. The two my_from_blob sites were compiled with the guard forced on,
> against a stub cuda_runtime.h. There is no CUDA toolkit here, so the
> aoti_inference block was not compiled; what I checked instead is that
> `<torch/torch.h>` is included unconditionally above it and does provide
> TORCH_CHECK, and that the block contained no TORCH_CHECK before this change, so
> the macro's availability there is an argument rather than a measurement. Every
> other converted line is unguarded.
>
> BUILD_TEST=0 does not prevent compiling these directly:
>
> aoti_inference/test.cpp syntax-only (needs gmock headers)
> aoti_custom_class.cpp syntax-only
> dataloader.cpp 87 tests passed
> test_custom_class_registrations.cpp compiles and links (no TESTs of its own)
> FileStoreTest.cpp syntax-only
> cpp_c10d_extension.cpp syntax-only (needs Python.h, pybind11)
> rng_extension.cpp syntax-only (same)
> my_from_blob_with_deleter.cpp syntax-only
> my_from_blob_with_lambda_deleter.cpp syntax-only
> OpenRegException.cpp syntax-only (needs the openreg headers)
>
> ```
> g++ -std=c++20 <includes> test/cpp/api/dataloader.cpp gtest-all.o gtest_main.o \
> -Lbuild/lib -lc10 -ltorch_cpu -lpthread && ./a.out
> [ PASSED ] 87 tests.
> ```
>
> Positive control on the dataloader annotation - the same file with the throw
> converted:
>
> ```
> [ FAILED ] DataLoaderTest.TestExceptionsArePropagatedFromWorkers
> ```
>
> So that annotation is required, not preference.
>
> ```
> lintrunner --take RAWTHROW --all-files
> spin lint
> ```
>
> Both clean; test/cpp and test/cpp_extensions go from 48 findings to 0.
> CLANGFORMAT rewrapped three of the new comments, applied via `spin fixlint`.
>
> One scope caveat worth stating plainly: "48 findings to 0" is about the linter's
> view, not the directories. RAWTHROW's include_patterns are `**/*.cpp` and
> `**/*.h`, so four raw throws here stay invisible - three std::system_error sites
> in `test/cpp/c10d/TestUtils.hpp` (two of which are literal copies of the throw
> this PR annotates in FileStoreTest.cpp) and a CUDA error-check macro in
> `test/cpp/aoti_abi_check/cuda/test_kernel_utils.cu`. A planned later phase
> widens include_patterns to .hpp/.cu and picks them up.
>
> One coupling to carry forward, in a file this PR touches:
> `aoti_inference/test.cpp:301` catches `const std::runtime_error&` around
> `update_constant_buffer` and matches "API call failed at" at :302. That throw comes from
> `torch/headeronly/util/shim_utils.h`, which is annotated and unconverted today -
> if a later phase converts it to TORCH_CHECK rather than STD_TORCH_CHECK, this
> catch stops matching.
>
> Not verified locally: the CUDA and distributed builds, and CLANGTIDY.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/194773
Approved by: https://github.com/ezyang
ghstack dependencies: #194379, #194772
Fixes#71069.
`SequentialImpl::clone()` rebuilt the container with the *unnamed* `push_back` overload, which
assigns each submodule the positional name `std::to_string(modules_.size())`. Every registered
name was replaced by its index, so cloning a named `Sequential` silently rewrote its
`state_dict` keys — `encoder.weight` became `0.weight` — breaking a save/load round trip
against the original module with no error.
`ModuleDictImpl::clone()` is unaffected because its `modules_` is an `OrderedDict` and it
clones through `insert(key, value)`. `Sequential`'s `modules_` is a plain `std::vector<AnyModule>`
and carries no names, so they have to come from the registered children. `clone()` now reads
them from the public `named_children()` and uses the named `push_back`.
This deliberately does not use `modules_[i].ptr()->name()`: that returns the demangled RTTI
type name, so a container holding two modules of the same type would produce duplicate keys.
### Test
`SequentialTest.ClonePreservesSubmoduleNames`, next to the existing `IsCloneable`. It asserts on
`named_children().keys()` and `named_parameters().keys()` rather than on `Module::name()` — the
latter is the *type* name and compares equal whether or not the bug is present, which is what
made the test in the earlier abandoned attempt (#90112) vacuous.
I cannot launch gtest binaries on my machine (Device Guard blocks unsigned executables), so the
same scenarios were run through a Python extension module against the real container. Before:
```
named clone : 0, 1
params clone : 0.weight, 0.bias
mixed clone : 0, 1, 2
unnamed clone : 0, 1, 2, 3
```
After:
```
named clone : encoder, act
params clone : encoder.weight, encoder.bias
mixed clone : first, 1, last
unnamed clone : 0, 1, 2, 3
```
An unnamed submodule keeps the positional name it was originally given rather than being
renumbered around the named ones, and a container with no names at all is unchanged.
### On numeric submodule names
@malfet asked this on the earlier attempt (#90112) and it was never answered, so:
```
numeric original : 2, 0, 1
numeric clone : 2, 0, 1
numeric clone^2 : 2, 0, 1
collision build : THREW on the original -> Submodule '1' already defined
```
Names round trip verbatim, so numeric names that do not match their position are preserved
rather than renumbered, and cloning is idempotent. A container whose auto-name would collide
with an explicit numeric one cannot be built at all: `register_module` inserts into an
`OrderedDict`, which throws on a duplicate key, so `Sequential({{"1", A}})` followed by an
unnamed `push_back` throws on the *original* long before `clone()` is reached. The proposed
behaviour is strictly closer to the original than today's renumbering.
Developed with AI assistance (Claude); I reviewed the change and the reasoning above.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/195107
Approved by: https://github.com/Skylion007
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
## Author Notes
Pretty mechanical, nothing special here.
## Agent Notes
Written by Claude Code:
> The two test directories are 48 findings, and they come apart cleanly: 43 are
> stubs and error checks that convert mechanically, and 5 are throws whose type is
> the point of the test they sit in. Getting those 5 wrong would leave the suite
> green while testing the wrong thing, which is the only real risk in this PR.
>
> The 43: `rng_extension.cpp` and `cpp_c10d_extension.cpp` are "not implemented" /
> "does not support" overrides (8 and 10); `aoti_inference/test.cpp` is CUDA API
> error checks (16); the rest are `aoti_custom_class.cpp`,
> `test_custom_class_registrations.cpp` (6, a toy stack machine's error paths) and
> the two `my_from_blob_*` extensions. None of their messages is asserted on
> anywhere - I grepped the tree for each, including the Python suites.
>
> The two `my_from_blob_*` sites get STD_TORCH_CHECK rather than TORCH_CHECK.
> Those TUs include only `torch/csrc/stable/*`, which reaches
> torch/headeronly/util/Exception.h but never c10/util/Exception.h, so TORCH_CHECK
> is not declared there at all. STD_TORCH_CHECK preserves the exception type
> exactly - it throws std::runtime_error, same as the raw throw - but not the
> message: like the serialization-codegen PR earlier in this stack, it prepends
> __func__, __FILE__ and __LINE__, so
>
> before: cudaMalloc failed
> after: my_from_blob_with_cuda_deleter, <path>/my_from_blob_with_deleter.cpp:73,
> cudaMalloc failed
>
> These are the only two sites in this PR where anything observable moves. Nothing
> asserts on either string, both are behind #ifdef LAE_USE_CUDA in a test
> extension, and STD_TORCH_CHECK is already the house style in the sibling
> libtorch_agn_2_* extensions (17 uses), so their messages are decorated the same
> way. The alternative was to leave these two raw and annotated; taking the
> decoration keeps the file consistent with its neighbours.
>
> The 5 that stay raw, with the reason on each site:
>
> ProcessGroupNCCLErrorsTest.cpp TestHeartbeatMonitor::runLoop, 16 lines
> above, catches std::runtime_error by name
> to set hasMonitorThreadCaughtError_. A
> c10::Error is not caught by that, and the
> throw happens on the monitor thread.
> dataloader.cpp The test rethrows e.original_exception and
> asserts it is still a std::invalid_argument
> - the point being that WorkerException
> round-trips the original type.
> parallel.cpp ParallelApplyRethrowsException is about a
> user exception escaping forward().
> FileStoreTest.cpp std::system_error carrying errno as a
> std::error_code.
> OpenRegException.cpp Reports a caller-supplied SourceLocation;
> a TORCH_CHECK there would stamp this
> file's own location instead.
>
> Two of those five are load-bearing in a way I could measure rather than argue,
> and the test plan shows the failures. `parallel.cpp` is the one that is not:
> its assertion is `ASSERT_THROWS_WITH(..., "Badness!")`, a substring match that
> would still pass after conversion. It is annotated for intent, not because it
> would break, and it seemed worth being explicit about that difference rather
> than implying all five were equally forced.
>
> The exclude list goes from 11 entries to 9.
>
> Test Plan:
>
> Half of this is CUDA or distributed and does not build here, so the two CUDA
> files - parallel.cpp and ProcessGroupNCCLErrorsTest.cpp - deliberately received
> comment-only edits, which is the safest possible change to code a configuration
> cannot compile.
>
> For the rest, what matters is whether the *changed lines* were compiled, not the
> files: three of them sit behind preprocessor guards that a plain CPU build skips
> straight over. `aoti_inference/test.cpp`'s 16 conversions are inside
> `#if defined(USE_CUDA) || defined(USE_ROCM)`, and the two `my_from_blob_*` sites
> are inside `#ifdef LAE_USE_CUDA`, which setup.py only defines when CUDA is
> available. The two my_from_blob sites were compiled with the guard forced on,
> against a stub cuda_runtime.h. There is no CUDA toolkit here, so the
> aoti_inference block was not compiled; what I checked instead is that
> `<torch/torch.h>` is included unconditionally above it and does provide
> TORCH_CHECK, and that the block contained no TORCH_CHECK before this change, so
> the macro's availability there is an argument rather than a measurement. Every
> other converted line is unguarded.
>
> BUILD_TEST=0 does not prevent compiling these directly:
>
> aoti_inference/test.cpp syntax-only (needs gmock headers)
> aoti_custom_class.cpp syntax-only
> dataloader.cpp 87 tests passed
> test_custom_class_registrations.cpp compiles and links (no TESTs of its own)
> FileStoreTest.cpp syntax-only
> cpp_c10d_extension.cpp syntax-only (needs Python.h, pybind11)
> rng_extension.cpp syntax-only (same)
> my_from_blob_with_deleter.cpp syntax-only
> my_from_blob_with_lambda_deleter.cpp syntax-only
> OpenRegException.cpp syntax-only (needs the openreg headers)
>
> ```
> g++ -std=c++20 <includes> test/cpp/api/dataloader.cpp gtest-all.o gtest_main.o \
> -Lbuild/lib -lc10 -ltorch_cpu -lpthread && ./a.out
> [ PASSED ] 87 tests.
> ```
>
> Positive control on the dataloader annotation - the same file with the throw
> converted:
>
> ```
> [ FAILED ] DataLoaderTest.TestExceptionsArePropagatedFromWorkers
> ```
>
> So that annotation is required, not preference.
>
> ```
> lintrunner --take RAWTHROW --all-files
> spin lint
> ```
>
> Both clean; test/cpp and test/cpp_extensions go from 48 findings to 0.
> CLANGFORMAT rewrapped three of the new comments, applied via `spin fixlint`.
>
> One scope caveat worth stating plainly: "48 findings to 0" is about the linter's
> view, not the directories. RAWTHROW's include_patterns are `**/*.cpp` and
> `**/*.h`, so four raw throws here stay invisible - three std::system_error sites
> in `test/cpp/c10d/TestUtils.hpp` (two of which are literal copies of the throw
> this PR annotates in FileStoreTest.cpp) and a CUDA error-check macro in
> `test/cpp/aoti_abi_check/cuda/test_kernel_utils.cu`. A planned later phase
> widens include_patterns to .hpp/.cu and picks them up.
>
> One coupling to carry forward, in a file this PR touches:
> `aoti_inference/test.cpp:301` catches `const std::runtime_error&` around
> `update_constant_buffer` and matches "API call failed at" at :302. That throw comes from
> `torch/headeronly/util/shim_utils.h`, which is annotated and unconverted today -
> if a later phase converts it to TORCH_CHECK rather than STD_TORCH_CHECK, this
> catch stops matching.
>
> Not verified locally: the CUDA and distributed builds, and CLANGTIDY.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/194773
Approved by: https://github.com/pytorchgreenlight
ghstack dependencies: #194378, #194379, #194772
Fixes#149120
Adds a `keepdim` parameter to `torch.cosine_similarity`, consistent with other reduction-like ops (`sum`, `norm`, `pairwise_distance`).
```python
>>> x1 = torch.randn(2, 3, 4)
>>> x2 = torch.randn(2, 3, 4)
>>> torch.cosine_similarity(x1, x2, dim=1).shape
torch.Size([2, 4])
>>> torch.cosine_similarity(x1, x2, dim=1, keepdim=True).shape
torch.Size([2, 1, 4])
```
**Changes:**
- `native_functions.yaml`: Add `bool keepdim=False` to `cosine_similarity` schema
- `Distance.cpp`: Pass `keepdim` to final `.sum(dim, keepdim)` call
- C++ API: Add `keepdim` to `CosineSimilarityOptions`, plumb through functional and module layers
- ONNX symbolic: Accept `keepdim` and pass to `keepdims_i` in reduce ops
- `torch/overrides.py`: Add `keepdim` to override lambda
- Tests: Add `keepdim=True` shape and value tests in Python (`test_nn.py`, OpInfo samples) and C++ (`modules.cpp`)
The default `keepdim=False` preserves full backward compatibility.
**Note:** Per the FC window policy, a follow-up PR will update the Python frontend (`F.cosine_similarity` docstring and `nn.CosineSimilarity` module) to expose `keepdim` after these C++ changes land.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/189654
Approved by: https://github.com/albanD
Co-authored-by: albanD via mergedog <desmaison.alban@gmail.com>
Fixes#154491
Replaces the internal assertion hit when dense tensor allocation receives `DeviceType::MKLDNN` with a user-facing `TORCH_CHECK_NOT_IMPLEMENTED` error. This leaves `Device("mkldnn")` parsing behavior intact while giving `torch.randn(..., device="mkldnn")` a clean error instead of an internal assert.
Test Plan:
- Added `TensorOptionsTest.MkldnnDeviceAllocationFailsCleanly`
- `git diff --check`
Note: I did not run the C++ test binary locally because this checkout is sparse/source-only and does not have a built PyTorch test binary.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/185711
Approved by: https://github.com/ezyang
# Fix: Division-by-Zero in `MultiheadAttentionImpl::reset()` When `num_heads == 0`
## Summary
Fixes#186237
Fixed a division-by-zero in `MultiheadAttentionImpl::reset()` triggered when `num_heads` is `0`.
Previously, `head_dim` was computed before any validation of `num_heads`:
```cpp
head_dim = options.embed_dim() / options.num_heads();
```
Constructing `MultiheadAttention` with `num_heads = 0` could invoke undefined behavior (integer division by zero / floating-point exception) before any validation logic was reached.
This PR adds a `TORCH_CHECK` to validate that `num_heads > 0` **before** performing the division, replacing the silent UB with a clear, actionable error message.
## Changes
- Added a `TORCH_CHECK(options.num_heads() > 0, ...)` guard in `MultiheadAttentionImpl::reset()` prior to the `head_dim` computation.
## Test Plan
Added a C++ API test verifying that constructing:
```cpp
MultiheadAttention(MultiheadAttentionOptions(0, 0))
```
throws a `c10::Error`.
**Before this change:** the test triggered a floating-point exception due to division by zero.
**After this change:** the test passes and the invalid configuration is reported via a proper error.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/186376
Approved by: https://github.com/drisspg
CPP20 introduced an rvalue overload to stringstream.str() . Without it, the string is copied out for the str() call every time. Now, we can steal the internal string buffer and return it directly through the rvalue overload. Mechanical find and replace assisted by codex.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/186552
Approved by: https://github.com/malfet
BE project hard-removing the deprecated named tensor feature to remove overhead and code bloat. This was nearly all done by wrestling with Claude Code, so it needs some personal review and probably needs to be split into multiple PRs to help reviewers.
The process was me absent-mindedly doing this throughout the week while working on other stuff:
```
me: remove named tensor along with all associated APIs, tests, and docs from the codebase
claude: <does stuff>
while not built:
me: <tries to build>
if error:
me: there are still remnants of named tensor (with build-error related example); remove all these
claude: <does more stuff, usually very good at generalizing from the example>
```
This wasn't particularly efficient (it built after ~27 iterations) but didn't require much thought or time from my end. The only part Claude Code did NOT do is add a couple now-necessary `#include <ATen/WrapDimUtils.h>` statements.
It builds! Testing is still a WIP.
Having Claude Code break this into manageable PRs, the breakdown is:
```
1. Remove named tensor tests and documentation
- test/test_namedtensor.py
- test/compiled_autograd_skips/TestNamedTensor.*
- test/dynamo_expected_failures/TestNamedTensor.*
- docs/source/named_tensor.md
- docs/source/name_inference.md
- Test file updates (test_ops.py, test_overrides.py, test_builtins.py, test_torch.py, etc.)
- functorch/op_analysis/annotated_ops
- Expect file updates (HasDecompTest)
- test/allowlist_for_publicAPI.json
2. Remove named tensor operators from native_functions.yaml
- All Dimname-based operator overloads
- align_tensors, align_as, align_to, rename, refine_names
- Factory function names variants (empty.names, zeros.names, etc.)
3. Remove Python named tensor APIs
- torch/_namedtensor_internals.py
- torch/_tensor.py (names property, rename methods)
- torch/functional.py (align_tensors)
- torch/overrides.py references
- torch/_dynamo/trace_rules.py references
- torch/multiprocessing/reductions.py references
4. Remove C++ named tensor infrastructure and build system
- Dimname type system (Dimname.h/cpp, python_dimname.h/cpp, python_arg_parser changes)
- Core infrastructure (NamedTensor.h/cpp, NamedTensorUtils.h/cpp, TensorNames.h/cpp)
- TensorImpl changes (NamedTensorMetaInterface)
- Structured kernels (names parameter from set_output_* and all call sites)
- Guards (NoNamesGuard, NamesMode, guard.reset() calls)
- Build system (CMakeLists.txt, build_variables.bzl, cmake/prioritized_text.txt)
- Codegen (torchgen/, tools/autograd/ templates)
```
Pull Request resolved: https://github.com/pytorch/pytorch/pull/173895
Approved by: https://github.com/malfet, https://github.com/guangyey, https://github.com/cota
BE project hard-removing the deprecated named tensor feature to remove overhead and code bloat. This was nearly all done by wrestling with Claude Code, so it needs some personal review and probably needs to be split into multiple PRs to help reviewers.
The process was me absent-mindedly doing this throughout the week while working on other stuff:
```
me: remove named tensor along with all associated APIs, tests, and docs from the codebase
claude: <does stuff>
while not built:
me: <tries to build>
if error:
me: there are still remnants of named tensor (with build-error related example); remove all these
claude: <does more stuff, usually very good at generalizing from the example>
```
This wasn't particularly efficient (it built after ~27 iterations) but didn't require much thought or time from my end. The only part Claude Code did NOT do is add a couple now-necessary `#include <ATen/WrapDimUtils.h>` statements.
It builds! Testing is still a WIP.
Having Claude Code break this into manageable PRs, the breakdown is:
```
1. Remove named tensor tests and documentation
- test/test_namedtensor.py
- test/compiled_autograd_skips/TestNamedTensor.*
- test/dynamo_expected_failures/TestNamedTensor.*
- docs/source/named_tensor.md
- docs/source/name_inference.md
- Test file updates (test_ops.py, test_overrides.py, test_builtins.py, test_torch.py, etc.)
- functorch/op_analysis/annotated_ops
- Expect file updates (HasDecompTest)
- test/allowlist_for_publicAPI.json
2. Remove named tensor operators from native_functions.yaml
- All Dimname-based operator overloads
- align_tensors, align_as, align_to, rename, refine_names
- Factory function names variants (empty.names, zeros.names, etc.)
3. Remove Python named tensor APIs
- torch/_namedtensor_internals.py
- torch/_tensor.py (names property, rename methods)
- torch/functional.py (align_tensors)
- torch/overrides.py references
- torch/_dynamo/trace_rules.py references
- torch/multiprocessing/reductions.py references
4. Remove C++ named tensor infrastructure and build system
- Dimname type system (Dimname.h/cpp, python_dimname.h/cpp, python_arg_parser changes)
- Core infrastructure (NamedTensor.h/cpp, NamedTensorUtils.h/cpp, TensorNames.h/cpp)
- TensorImpl changes (NamedTensorMetaInterface)
- Structured kernels (names parameter from set_output_* and all call sites)
- Guards (NoNamesGuard, NamesMode, guard.reset() calls)
- Build system (CMakeLists.txt, build_variables.bzl, cmake/prioritized_text.txt)
- Codegen (torchgen/, tools/autograd/ templates)
```
Pull Request resolved: https://github.com/pytorch/pytorch/pull/173895
Approved by: https://github.com/malfet, https://github.com/guangyey
BE project hard-removing the deprecated named tensor feature to remove overhead and code bloat. This was nearly all done by wrestling with Claude Code, so it needs some personal review and probably needs to be split into multiple PRs to help reviewers.
The process was me absent-mindedly doing this throughout the week while working on other stuff:
```
me: remove named tensor along with all associated APIs, tests, and docs from the codebase
claude: <does stuff>
while not built:
me: <tries to build>
if error:
me: there are still remnants of named tensor (with build-error related example); remove all these
claude: <does more stuff, usually very good at generalizing from the example>
```
This wasn't particularly efficient (it built after ~27 iterations) but didn't require much thought or time from my end. The only part Claude Code did NOT do is add a couple now-necessary `#include <ATen/WrapDimUtils.h>` statements.
It builds! Testing is still a WIP.
Having Claude Code break this into manageable PRs, the breakdown is:
```
1. Remove named tensor tests and documentation
- test/test_namedtensor.py
- test/compiled_autograd_skips/TestNamedTensor.*
- test/dynamo_expected_failures/TestNamedTensor.*
- docs/source/named_tensor.md
- docs/source/name_inference.md
- Test file updates (test_ops.py, test_overrides.py, test_builtins.py, test_torch.py, etc.)
- functorch/op_analysis/annotated_ops
- Expect file updates (HasDecompTest)
- test/allowlist_for_publicAPI.json
2. Remove named tensor operators from native_functions.yaml
- All Dimname-based operator overloads
- align_tensors, align_as, align_to, rename, refine_names
- Factory function names variants (empty.names, zeros.names, etc.)
3. Remove Python named tensor APIs
- torch/_namedtensor_internals.py
- torch/_tensor.py (names property, rename methods)
- torch/functional.py (align_tensors)
- torch/overrides.py references
- torch/_dynamo/trace_rules.py references
- torch/multiprocessing/reductions.py references
4. Remove C++ named tensor infrastructure and build system
- Dimname type system (Dimname.h/cpp, python_dimname.h/cpp, python_arg_parser changes)
- Core infrastructure (NamedTensor.h/cpp, NamedTensorUtils.h/cpp, TensorNames.h/cpp)
- TensorImpl changes (NamedTensorMetaInterface)
- Structured kernels (names parameter from set_output_* and all call sites)
- Guards (NoNamesGuard, NamesMode, guard.reset() calls)
- Build system (CMakeLists.txt, build_variables.bzl, cmake/prioritized_text.txt)
- Codegen (torchgen/, tools/autograd/ templates)
```
Pull Request resolved: https://github.com/pytorch/pytorch/pull/173895
Approved by: https://github.com/malfet, https://github.com/guangyey
This PR implements a native C++ API to toggle scientific notation in tensor printing (fixes#40613). Previously, C++ users could not disable scientific notation, unlike Python's `torch.set_printoptions(sci_mode=False)`.
**Key Changes:**
- Added `torch::set_print_sci_mode(bool)` for native C++ control
- Updates core ATen tensor formatting logic
- Works without Python dependencies
- Includes GTest coverage (`PrintSciModeTest`)
**Usage Example:**
```cpp
auto t = torch::tensor({0.00001, 100000.0});
torch::set_print_sci_mode(false); // disable scientific notation
std::cout << t << std::endl; // Output: 0.0000, 100000.0000
```
**Testing:**
```bash
cd build
cmake --build . --target test_api -j4
./bin/test_api --gtest_filter="PrintSciModeTest*"
```
Expected Output:
```
With sci_mode=true: 1.0000e-05, 1.0000e+05
With sci_mode=false: 0.0000, 100000.0000
```
Pull Request resolved: https://github.com/pytorch/pytorch/pull/173321
Approved by: https://github.com/ezyang
Co-authored-by: Edward Yang <ezyang@meta.com>
Replace std::shared_ptr<Node> with c10::intrusive_ptr<Node> throughout the autograd system. Node now inherits from c10::intrusive_ptr_target instead of std::enable_shared_from_this<Node>.
**BC Breaking**:
This changes the signature of `Tensor::grad_fn`. Accesses to `Tensor.grad_fn()` should change from `std::shared_ptr<Node>` to `c10::intrusive_ptr<Node>`. Similarly, construction of a C++ autograd function should change from:
```c++
std::shared_ptr<CustomCppNode> node(new CustomCppNode(), torch::autograd::deleteNode);
```
to
```c++
auto node = c10::make_intrusive<CustomCppNode>();
```
Authored with Claude.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/181139
Approved by: https://github.com/albanD
Fixes#141884
This fixes the issue for all optimizers and parameter options.
A member function `overwrite_from` is added to the optimizer base class. Each optimizer then implements this function for comparing their accepted parameters to defaults. A SFINAE approach to handle the different optimizer parameters generically (in optimizer.h only) was evaluated, but I think this is easier to review and maintain.
This mirrors the Python API up to one edge case. An example of the edge case is provided below.
Python can distinguish between 1) Key not present in dict = "not specified" and 2) Key present in dict = "explicitly set". The C++ implementation cannot.
The issue hinges on whether or not to track if a particular parameter was set by the user explicitly or not (discrepancy in the case when the constructor default is explicitly passed in).
To track this seems like it will take more intervention than would be worth it (modify TORCH_ARG to keep track, use std::optional for the parameter types, use bitset tracking) and was not pursued in the current PR. I'm happy to alter the design if appropriate.
### Example of edge case hinging on CONSTRUCTOR DEFAULTS vs OPTIMIZER DEFAULTS
1. CONSTRUCTOR DEFAULTS:
These are the values you get when calling AdamOptions()
AdamOptions().lr() = 0.001
AdamOptions().weight_decay() = 0
AdamOptions().eps() = 1e-08
2. OPTIMIZER DEFAULTS:
These are the values the user chose when creating the optimizer
User's optimizer defaults:
optimizer.lr() = 0.005
optimizer.weight_decay() = 0.1
optimizer.eps() = 1e-07
3. THE PROBLEM SCENARIO:
User wants to add a parameter group with explicit weight_decay=0.0
User sets: weight_decay(0)
4. THE CONFUSION:
Constructor default weight_decay: 0
User's explicit weight_decay: 0
Are they equal? YES
Since they're equal, our overwrite_from() logic thinks:
"User didn't set weight_decay explicitly, use optimizer default"
5. CURRENT BEHAVIOR:
Final weight_decay: 0.1
User expected: 0
Match? ❌ NO
=== KEY INSIGHT ===
Constructor defaults are built into the C++ class definition.
Optimizer defaults are chosen by the user at runtime. We want to respect the user intention.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/161825
Approved by: https://github.com/janeyx99
When bicubic interpolation was added to grid_sampler in #44780, `GridSampleFuncOptions` was not updated to allow a user to use bicubic mode in LibTorch, even though the function could handle it. This PR fixes the parity such that LibTorch's `torch::nn::functional::grid_sample` behaves the same as PyTorch's `torch.nn.functional.grid_sample`.
Existing users can directly use `torch::grid_sampler` but must know what int to pass for the interpolation (2 for bicubic) and padding mode parameters, which is not ideal.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/150817
Approved by: https://github.com/Skylion007
Similar to #140425, we are making the implementation usable via header-only code sharing.
Review note: #62546 by @yanbing-j removed expm1 usage from this path. I don't know why and expm1 should be more efficient, so I've put it back. Please let me know if there is a good reason I shouldn't.
Testing: existing correctness tests should cover.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/149673
Approved by: https://github.com/cyyever, https://github.com/Skylion007
Edward Z. Yang