mirror of
https://github.com/pytorch/pytorch.git
synced 2026-10-28 18:56:51 +08:00
albanD
f6a98bbf71
## 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
123 lines
3.6 KiB
C++
123 lines
3.6 KiB
C++
#include "cpp_c10d_extension.hpp"
|
|
|
|
#include <map>
|
|
|
|
namespace c10d {
|
|
|
|
ProcessGroupTest::WorkTest::~WorkTest() {}
|
|
|
|
bool ProcessGroupTest::WorkTest::isCompleted() {
|
|
return true;
|
|
}
|
|
|
|
bool ProcessGroupTest::WorkTest::isSuccess() const {
|
|
return true;
|
|
}
|
|
|
|
bool ProcessGroupTest::WorkTest::wait(std::chrono::milliseconds /* unused */) {
|
|
return true;
|
|
}
|
|
|
|
ProcessGroupTest::ProcessGroupTest(int rank, int size)
|
|
: ProcessGroup(rank, size) {}
|
|
|
|
ProcessGroupTest::~ProcessGroupTest() {}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::broadcast(
|
|
std::vector<at::Tensor>& tensors,
|
|
const BroadcastOptions& opts) {
|
|
return c10::make_intrusive<ProcessGroupTest::WorkTest>();
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::allreduce(
|
|
std::vector<at::Tensor>& tensors,
|
|
const AllreduceOptions& opts) {
|
|
return c10::make_intrusive<ProcessGroupTest::WorkTest>();
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::allreduce_coalesced(
|
|
std::vector<at::Tensor>& tensors,
|
|
const AllreduceCoalescedOptions& opts) {
|
|
TORCH_CHECK(false, "ProcessGroupTest does not support allreduce_coalesced");
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::reduce(
|
|
std::vector<at::Tensor>& tensors,
|
|
const ReduceOptions& opts) {
|
|
TORCH_CHECK(false, "ProcessGroupTest does not support reduce");
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::allgather(
|
|
std::vector<std::vector<at::Tensor>>& outputTensors,
|
|
std::vector<at::Tensor>& inputTensors,
|
|
const AllgatherOptions& opts) {
|
|
TORCH_CHECK(false, "ProcessGroupTest does not support allgather");
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::_allgather_base(
|
|
at::Tensor& outputBuffer,
|
|
at::Tensor& inputBuffer,
|
|
const AllgatherOptions& opts) {
|
|
TORCH_CHECK(false, "ProcessGroupTest does not support _allgather_base");
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::barrier(
|
|
const BarrierOptions& opts) {
|
|
return c10::make_intrusive<ProcessGroupTest::WorkTest>();
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::gather(
|
|
std::vector<std::vector<at::Tensor>>& outputTensors,
|
|
std::vector<at::Tensor>& inputTensors,
|
|
const GatherOptions& opts) {
|
|
TORCH_CHECK(false, "ProcessGroupTest does not support gather");
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::scatter(
|
|
std::vector<at::Tensor>& outputTensors,
|
|
std::vector<std::vector<at::Tensor>>& inputTensors,
|
|
const ScatterOptions& opts) {
|
|
TORCH_CHECK(false, "ProcessGroupTest does not support scatter");
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::reduce_scatter(
|
|
std::vector<at::Tensor>& outputTensors,
|
|
std::vector<std::vector<at::Tensor>>& inputTensors,
|
|
const ReduceScatterOptions& opts) {
|
|
TORCH_CHECK(false, "ProcessGroupTest does not support reduce_scatter");
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::send(
|
|
std::vector<at::Tensor>& tensors,
|
|
int dstRank,
|
|
int tag) {
|
|
TORCH_CHECK(false, "ProcessGroupTest does not support send");
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::recv(
|
|
std::vector<at::Tensor>& tensors,
|
|
int srcRank,
|
|
int tag) {
|
|
TORCH_CHECK(false, "ProcessGroupTest does not support recv");
|
|
}
|
|
|
|
c10::intrusive_ptr<Work> ProcessGroupTest::recvAnysource(
|
|
std::vector<at::Tensor>& tensor,
|
|
int tag) {
|
|
TORCH_CHECK(false, "ProcessGroupTest does not support recvAnysource");
|
|
}
|
|
|
|
c10::intrusive_ptr<ProcessGroup> ProcessGroupTest::createProcessGroupTest(
|
|
const c10::intrusive_ptr<::c10d::Store>& store,
|
|
int rank,
|
|
int size,
|
|
const std::chrono::duration<float>& timeout) {
|
|
return c10::make_intrusive<ProcessGroupTest>(rank, size);
|
|
}
|
|
|
|
PYBIND11_MODULE(TORCH_EXTENSION_NAME, m) {
|
|
m.def("createProcessGroupTest", &ProcessGroupTest::createProcessGroupTest);
|
|
}
|
|
|
|
} // namespace c10d
|