Skip to content

Commit

Permalink
Improve error messages for operator registration API (#47636)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: #47636

Previously:
```
terminate called after throwing an instance of 'c10::Error'
  what():  *cpp_signature == cpp_signature_->signature INTERNAL ASSERT FAILED at "caffe2/aten/src/ATen/core/dispatch/OperatorEntry.cpp":92, please report a bug to PyTorch. Tried to register a kernel (registered at buck-out/dev/gen/caffe2/generate-code/autograd/generated/TraceType_2.cpp:9847) for operator aten::div.out (registered at buck-out/dev/gen/caffe2/aten/gen_aten=TypeDefault.cpp/TypeDefault.cpp:3541) for dispatch key Tracer, but the C++ function signature at::Tensor& (at::Tensor const&, at::Tensor const&, at::Tensor&) mismatched with a previous kernel (registered at buck-out/dev/gen/caffe2/aten/gen_aten=CPUType.cpp/CPUType.cpp:2166) that had the signature at::Tensor& (at::Tensor&, at::Tensor const&, at::Tensor const&)
```
Now:
```
terminate called after throwing an instance of 'c10::Error'
  what():  *cpp_signature == cpp_signature_->signature INTERNAL ASSERT FAILED at "caffe2/aten/src/ATen/core/dispatch/OperatorEntry.cpp":96, please report a bug to PyTorch.
Mismatch in kernel C++ signatures
  operator: aten::div.out(Tensor self, Tensor other, *, Tensor(a!) out) -> (Tensor(a!))
    registered at buck-out/dev/gen/caffe2/aten/gen_aten=TypeDefault.cpp/TypeDefault.cpp:3541
  kernel 1: at::Tensor& (at::Tensor&, at::Tensor const&, at::Tensor const&)
    dispatch key: CPU
    registered at buck-out/dev/gen/caffe2/aten/gen_aten=CPUType.cpp/CPUType.cpp:2166
  kernel 2: at::Tensor& (at::Tensor const&, at::Tensor const&, at::Tensor&)
    dispatch key: Tracer
    registered at buck-out/dev/gen/caffe2/generate-code/autograd/generated/TraceType_2.cpp:9847
```

Previously:
```
W1109 13:38:52.464170 1644302 OperatorEntry.cpp:117] Warning: Registering a kernel (registered at caffe2/torch/csrc/autograd/VariableTypeManual.cpp:310) for operator aten::_backward (registered at buck-out/dev/gen/caffe2/aten/gen_aten=TypeDefault.cpp/TypeDefault.cpp:3549) for dispatch key Autograd that overwrote a previously registered kernel (registered at caffe2/torch/csrc/autograd/VariableTypeManual.cpp:310) with the same dispatch key for the same operator. (function registerKernel)
```
Now:
```
W1109 13:49:40.501817 1698959 OperatorEntry.cpp:118] Warning: Overriding a previously registered kernel for the same operator and the same dispatch key
  operator: aten::_backward(Tensor self, Tensor[] inputs, Tensor? gradient=None, bool? retain_graph=None, bool create_graph=False) -> ()
    registered at buck-out/dev/gen/caffe2/aten/gen_aten=TypeDefault.cpp/TypeDefault.cpp:3549
  dispatch key: Autograd
  previous kernel: registered at caffe2/torch/csrc/autograd/VariableTypeManual.cpp:310
       new kernel: registered at caffe2/torch/csrc/autograd/VariableTypeManual.cpp:310 (function registerKernel)
```

Previously:
```
terminate called after throwing an instance of 'c10::Error'
  what():  In registration for dummy_library::dummy_op: expected schema of operator to be "dummy_library::dummy_op(Tensor a) -> (Tensor)" (registered at caffe2/torch/csrc/autograd/VariableTypeManual.cpp:298), but got inferred schema "(Tensor _0) -> ()" (registered at caffe2/torch/csrc/autograd/VariableTypeManual.cpp:298). The number of returns is different. 1 vs 0
```
Now:
```
terminate called after throwing an instance of 'c10::Error'
  what():  Inferred operator schema for a C++ kernel function doesn't match the expected function schema.
  operator: dummy_library::dummy_op
  expected schema: dummy_library::dummy_op(Tensor a) -> (Tensor)
    registered at caffe2/torch/csrc/autograd/VariableTypeManual.cpp:298
  inferred schema: (Tensor _0) -> ()
    registered at caffe2/torch/csrc/autograd/VariableTypeManual.cpp:298
  reason: The number of returns is different. 1 vs 0
````

Previously:
```
terminate called after throwing an instance of 'c10::Error'
  what():  !cpp_signature_.has_value() || (CppSignature::make<FuncType>() == cpp_signature_->signature) INTERNAL ASSERT FAILED at "caffe2/aten/src/ATen/core/dispatch/OperatorEntry.h":170, please report a bug to PyTorch. Tried to access operator _test::dummy with a wrong signature. Accessed with void (at::Tensor, long) but the operator was registered with void (at::Tensor) (schema: registered by RegisterOperators, kernel: registered by RegisterOperators) This likely happened in a call to OperatorHandle::typed<Return (Args...)>(). Please make sure that the function signature matches the signature in the operator registration call.
```
Now:
```
terminate called after throwing an instance of 'c10::Error'
  what():  !cpp_signature_.has_value() || (CppSignature::make<FuncType>() == cpp_signature_->signature) INTERNAL ASSERT FAILED at "caffe2/aten/src/ATen/core/dispatch/OperatorEntry.h":169, please report a bug to PyTorch.
Tried to access or call an operator with a wrong signature.
  operator: _test::dummy(Tensor dummy) -> ()
    registered by RegisterOperators
  correct signature:  void (at::Tensor)
    registered by RegisterOperators
  accessed/called as: void (at::Tensor, long)
This likely happened in a call to OperatorHandle::typed<Return (Args...)>(). Please make sure that the function signature matches the signature in the operator registration call.
```
ghstack-source-id: 116359052

Test Plan: waitforsandcastle

Reviewed By: ezyang

Differential Revision: D24846523

fbshipit-source-id: 0ce7d487b725bfbdf2261e36027cb34ef50c1fea
  • Loading branch information
@smessmer @facebook-github-bot
smessmer authored and facebook-github-bot committed Nov 11, 2020
1 parent 05a76ed commit 7864ae9
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 43 deletions.
41 changes: 25 additions & 16 deletions aten/src/ATen/core/dispatch/OperatorEntry.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,13 @@ namespace {
c10::optional<std::string> schema_difference = findSchemaDifferences(from_def, inferred);
if (schema_difference.has_value()) {
TORCH_CHECK(false,
"In registration for ", toString(name), ": expected schema of operator to be \"", toString(from_def), "\" (", from_def_debug, "), ",
"but got inferred schema \"", toString(inferred), "\" (", inferred_debug, "). ",
*schema_difference);
"Inferred operator schema for a C++ kernel function doesn't match the expected function schema.\n"
" operator: ", toString(name), "\n",
" expected schema: ", toString(from_def), "\n",
" ", from_def_debug, "\n",
" inferred schema: ", toString(inferred), "\n",
" ", inferred_debug, "\n",
" reason: ", *schema_difference);
}
}
} // anonymous namespace
Expand Down Expand Up @@ -83,15 +87,19 @@ std::list<AnnotatedKernel>::iterator OperatorEntry::registerKernel(
// that would also invalidate the old TypedOperatorHandles.
if (cpp_signature.has_value()) {
if (cpp_signature_.has_value()) {
TORCH_INTERNAL_ASSERT(*cpp_signature == cpp_signature_->signature,
"Tried to register a kernel (", debug, ") for operator ", name_," (",
(this->schema_.has_value() ? this->schema_->debug : "no debug info"),
") for dispatch key ", toString(dispatch_key), ", but the C++ function signature ",
cpp_signature->name(), " mismatched with a previous kernel (", cpp_signature_->debug,
") that had the signature ", cpp_signature_->signature.name()
TORCH_CHECK(*cpp_signature == cpp_signature_->signature,
"\nMismatch in kernel C++ signatures\n",
" operator: ", (this->schema_.has_value() ? toString(this->schema_->schema) : toString(name_)), "\n",
" ", (this->schema_.has_value() ? this->schema_->debug : "no debug info"), "\n",
" kernel 1: ", cpp_signature_->signature.name(), "\n",
" dispatch key: ", toString(cpp_signature_->dispatch_key), "\n",
" ", cpp_signature_->debug, "\n",
" kernel 2: ", cpp_signature->name(), "\n",
" dispatch key: ", toString(dispatch_key), "\n",
" ", debug, "\n"
);
} else {
cpp_signature_ = CppSignatureWithDebug { *cpp_signature, debug };
cpp_signature_ = CppSignatureWithDebug { *cpp_signature, debug, dispatch_key };
}
}

Expand All @@ -105,12 +113,13 @@ std::list<AnnotatedKernel>::iterator OperatorEntry::registerKernel(
auto& k = dispatch_key.has_value() ? kernels_[*dispatch_key] : kernels_[DispatchKey::Math];

if (k.size() > 0) {
TORCH_WARN("Registering a kernel (", debug, ") for operator ", name_, " (",
(this->schema_.has_value() ? this->schema_->debug : "no debug info"),
") for dispatch key ", toString(dispatch_key),
" that overwrote a previously registered kernel (",
(cpp_signature_.has_value() ? cpp_signature_->debug : "no debug info"),
") with the same dispatch key for the same operator.");
TORCH_WARN("Overriding a previously registered kernel for the same operator and the same dispatch key\n",
" operator: ", (schema_.has_value() ? toString(schema_->schema) : toString(name_)), "\n",
" ", (this->schema_.has_value() ? this->schema_->debug : "no debug info"), "\n",
" dispatch key: ", toString(dispatch_key), "\n",
" previous kernel: ", (cpp_signature_.has_value() ? cpp_signature_->debug : "no debug info"), "\n",
" new kernel: ", debug
);
}

if (manuallyBoxedKernel_.has_value()) {
Expand Down
20 changes: 10 additions & 10 deletions aten/src/ATen/core/dispatch/OperatorEntry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,15 @@ class CAFFE2_API OperatorEntry final {
// Asserts that the given FuncType is correct for calling this operator in an unboxed way.
template<class FuncType>
void assertSignatureIsCorrect() {
TORCH_INTERNAL_ASSERT(!cpp_signature_.has_value() || (CppSignature::make<FuncType>() == cpp_signature_->signature),
"Tried to access operator ", name_, " with a wrong signature. Accessed with ",
CppSignature::make<FuncType>().name(),
" but the operator was registered with ",
cpp_signature_->signature.name(),
" (schema: ",
(schema_.has_value() ? schema_->debug : "unknown debug info"),
", kernel: ",
cpp_signature_->debug,
") This likely happened in a call to OperatorHandle::typed<Return (Args...)>(). Please make sure that the function signature matches the signature in the operator registration call."
TORCH_CHECK(!cpp_signature_.has_value() || (CppSignature::make<FuncType>() == cpp_signature_->signature),
"\nTried to access or call an operator with a wrong signature.\n",
" operator: ", (schema_.has_value() ? toString(schema_->schema) : toString(name_)), "\n",
" ", (schema_.has_value() ? schema_->debug : "unknown debug info"), "\n",
" correct signature: ", cpp_signature_->signature.name(), "\n",
" ", cpp_signature_->debug, "\n",
" accessed/called as: ", CppSignature::make<FuncType>().name(), "\n",
"This likely happened in a call to OperatorHandle::typed<Return (Args...)>(). ",
"Please make sure that the function signature matches the signature in the operator registration call."
);
}

Expand Down Expand Up @@ -241,6 +240,7 @@ class CAFFE2_API OperatorEntry final {
struct CppSignatureWithDebug {
CppSignature signature;
std::string debug;
c10::optional<DispatchKey> dispatch_key;
};
c10::optional<CppSignatureWithDebug> cpp_signature_;

Expand Down
32 changes: 15 additions & 17 deletions aten/src/ATen/core/op_registration/op_registration_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,9 @@ TEST(OperatorRegistrationTest, givenMultipleKernelsWithSameDispatchKey_whenRegis
testing::internal::CaptureStderr();
c10::RegisterOperators().op("_test::dummy(Tensor dummy) -> ()", c10::RegisterOperators::options().kernel<DummyKernel>(c10::DispatchKey::CPU));
std::string output = testing::internal::GetCapturedStderr();
EXPECT_THAT(output, testing::HasSubstr("_test::dummy"));
EXPECT_THAT(output, testing::HasSubstr("CPU"));
EXPECT_THAT(output, testing::HasSubstr("overwrote a previously registered kernel "));
EXPECT_THAT(output, testing::HasSubstr(" with the same dispatch key for the same operator"));
EXPECT_THAT(output, testing::HasSubstr("Overriding a previously registered kernel for the same operator and the same dispatch key"));
EXPECT_THAT(output, testing::HasSubstr("operator: _test::dummy(Tensor dummy) -> ()"));
EXPECT_THAT(output, testing::HasSubstr("dispatch key: CPU"));
}

TEST(OperatorRegistrationTest, givenMultipleKernelsWithSameDispatchKey_whenRegisteringInSameOpCall_thenFails) {
Expand Down Expand Up @@ -347,10 +346,9 @@ TEST(OperatorRegistrationTest, givenMultipleCatchallKernels_whenRegistering_then
testing::internal::CaptureStderr();
c10::RegisterOperators().op("_test::dummy(Tensor dummy) -> ()", c10::RegisterOperators::options().catchAllKernel<DummyKernel>());
std::string output = testing::internal::GetCapturedStderr();
EXPECT_THAT(output, testing::HasSubstr("_test::dummy"));
EXPECT_THAT(output, testing::HasSubstr("catch all"));
EXPECT_THAT(output, testing::HasSubstr("overwrote a previously registered kernel "));
EXPECT_THAT(output, testing::HasSubstr(" with the same dispatch key for the same operator"));
EXPECT_THAT(output, testing::HasSubstr("Overriding a previously registered kernel for the same operator and the same dispatch key"));
EXPECT_THAT(output, testing::HasSubstr("operator: _test::dummy(Tensor dummy) -> ()"));
EXPECT_THAT(output, testing::HasSubstr("dispatch key: (catch all)"));
}

TEST(OperatorRegistrationTest, givenMultipleCatchallKernels_whenRegisteringInSameOpCall_thenFails) {
Expand Down Expand Up @@ -703,7 +701,7 @@ TEST(OperatorRegistrationTest, whenRegisteringMismatchingKernelsInSameOpCall_the
auto registrar1 = c10::RegisterOperators().op("_test::dummy", c10::RegisterOperators::options()
.kernel<DummyKernelWithIntParam>(c10::DispatchKey::CPU)
.kernel<MockKernel>(c10::DispatchKey::CUDA, &called_kernel));
}, "mismatched with a previous kernel");
}, "Mismatch in kernel C++ signatures");
}

void backend_fallback_kernel(const c10::OperatorHandle& op, c10::Stack* stack) {
Expand Down Expand Up @@ -946,7 +944,7 @@ TEST(OperatorRegistrationTest, givenLambdaKernel_whenRegisteringWithMismatchingC
expectThrows<c10::Error>([] {
auto registrar = c10::RegisterOperators().op("_test::dummy", c10::RegisterOperators::options()
.kernel(DispatchKey::CPU, [] (const int64_t&) {}));
}, "mismatched with a previous kernel");
}, "Mismatch in kernel C++ signatures");
}

TEST(OperatorRegistrationTest, givenLambdaKernel_whenRegisteringCatchAllAndBackendWithMismatchingCppSignatures_thenFails) {
Expand All @@ -955,7 +953,7 @@ TEST(OperatorRegistrationTest, givenLambdaKernel_whenRegisteringCatchAllAndBacke
expectThrows<c10::Error>([] {
auto registrar = c10::RegisterOperators().op("_test::dummy", c10::RegisterOperators::options()
.kernel(DispatchKey::CPU, [] (const int64_t&) {}));
}, "mismatched with a previous kernel");
}, "Mismatch in kernel C++ signatures");
}

TEST(OperatorRegistrationTest, givenLambdaKernel_whenRegisteringBackendAndCatchAllWithMismatchingCppSignatures_thenFails) {
Expand All @@ -964,7 +962,7 @@ TEST(OperatorRegistrationTest, givenLambdaKernel_whenRegisteringBackendAndCatchA
expectThrows<c10::Error>([] {
auto registrar = c10::RegisterOperators().op("_test::dummy", c10::RegisterOperators::options()
.catchAllKernel([] (const int64_t&) {}));
}, "mismatched with a previous kernel");
}, "Mismatch in kernel C++ signatures");
}

TEST(OperatorRegistrationTest, givenLambdaKernel_whenAccessingWithMismatchingCppSignatures_thenFails) {
Expand All @@ -973,7 +971,7 @@ TEST(OperatorRegistrationTest, givenLambdaKernel_whenAccessingWithMismatchingCpp
expectThrows<c10::Error>([] {
c10::Dispatcher::singleton().findSchemaOrThrow("_test::dummy", "")
.typed<void(const int64_t&)>();
}, "Tried to access operator _test::dummy with a wrong signature");
}, "Tried to access or call an operator with a wrong signature.\n operator: _test::dummy(int _0) -> ()");
}

TEST(OperatorRegistrationTest, givenLambdaKernel_whenAccessingCatchAllWithMismatchingCppSignatures_thenFails) {
Expand All @@ -982,7 +980,7 @@ TEST(OperatorRegistrationTest, givenLambdaKernel_whenAccessingCatchAllWithMismat
expectThrows<c10::Error>([] {
c10::Dispatcher::singleton().findSchemaOrThrow("_test::dummy", "")
.typed<void(const int64_t&)>();
}, "Tried to access operator _test::dummy with a wrong signature");
}, "Tried to access or call an operator with a wrong signature.\n operator: _test::dummy(int _0) -> ()");
}

TEST(OperatorRegistrationTest, givenTorchLibrary_whenRegisteringWithMismatchingCppSignatures_thenFails) {
Expand All @@ -991,7 +989,7 @@ TEST(OperatorRegistrationTest, givenTorchLibrary_whenRegisteringWithMismatchingC
m.impl("dummy", DispatchKey::CPU, [] (int64_t) {});
expectThrows<c10::Error>([&] {
m.impl("dummy", DispatchKey::CUDA, [] (const int64_t&) {});
}, "mismatched with a previous kernel");
}, "Mismatch in kernel C++ signatures");
}

TEST(OperatorRegistrationTest, givenTorchLibrary_whenAccessingWithMismatchingCppSignatures_thenFails) {
Expand All @@ -1001,7 +999,7 @@ TEST(OperatorRegistrationTest, givenTorchLibrary_whenAccessingWithMismatchingCpp
expectThrows<c10::Error>([] {
c10::Dispatcher::singleton().findSchemaOrThrow("_test::dummy", "")
.typed<void(const int64_t&)>();
}, "Tried to access operator _test::dummy with a wrong signature");
}, "Tried to access or call an operator with a wrong signature.\n operator: _test::dummy(int a) -> ()");
}

TEST(OperatorRegistrationTest, givenTorchLibrary_whenAccessingCatchAllWithMismatchingCppSignatures_thenFails) {
Expand All @@ -1010,7 +1008,7 @@ TEST(OperatorRegistrationTest, givenTorchLibrary_whenAccessingCatchAllWithMismat
expectThrows<c10::Error>([] {
c10::Dispatcher::singleton().findSchemaOrThrow("_test::dummy", "")
.typed<void(const int64_t&)>();
}, "Tried to access operator _test::dummy with a wrong signature");
}, "Tried to access or call an operator with a wrong signature.\n operator: _test::dummy(int a) -> ()");
}

/**
Expand Down

0 comments on commit 7864ae9

Please sign in to comment.