schema_matching.cpp: improve error messages.

Mikhail Zolotukhin · facebook-github-bot · commit 96910251e093 · 2019-06-12T12:43:12.000-07:00
Summary: Pull Request resolved: #21141 Differential Revision: D15769066 Pulled By: ZolotukhinM fbshipit-source-id: 5853e0360581c44e42b068add3bf2bc68e671b2b
diff --git a/aten/src/ATen/core/function_schema.h b/aten/src/ATen/core/function_schema.h
@@ -64,7 +64,7 @@ struct Argument {
           "because it was not annotated with an explicit type.\n");
     }
     return c10::str(
-        "expected a value of type '",
+        "Expected a value of type '",
         type()->python_str(),
         "' for argument '",
         name(),
diff --git a/test/cpp/api/jit.cpp b/test/cpp/api/jit.cpp
@@ -62,7 +62,7 @@ TEST(TorchScriptTest, TestNestedIValueModuleArgMatching) {
   } catch (const c10::Error& error) {
     AT_ASSERT(
         std::string(error.what_without_backtrace())
-            .find("nested_loop() expected a value of type 'List[List[Tensor]]'"
+            .find("nested_loop() Expected a value of type 'List[List[Tensor]]'"
                   " for argument 'a' but instead found type "
                   "'List[List[List[t]]]'") == 0);
   };
@@ -81,7 +81,7 @@ TEST(TorchScriptTest, TestNestedIValueModuleArgMatching) {
     //so the error message is not helpful here.
     AT_ASSERT(
         std::string(error.what_without_backtrace())
-            .find("nested_loop() expected a value of type "
+            .find("nested_loop() Expected a value of type "
                   "'List[List[Tensor]]' for argument 'a' but "
                   "instead found type 'List[List[Tensor]]'") == 0);
   };
diff --git a/test/test_jit.py b/test/test_jit.py
@@ -2904,7 +2904,7 @@ def test_sequence_parsing(self):
             ("return [x x]", "expected ]"),
             ("return x, x,", True),
             ("return bar(x, x,)", True),
-            ("return bar()", "argument x not provided"),
+            ("return bar()", "Argument x not provided"),
             ("for a, b, in x, x,:\n        pass", "List of iterables"),
             ("a, b, = x, x,\n    return a + b", True)
         ]
@@ -4509,7 +4509,7 @@ def comp(l1, l2):
         self.assertEqual(comp([1, 2, 3], [4, 5]), [3, 6, 9, 6, 7])
 
     def test_comprehensions_wrong_expr_type(self):
-        with self.assertRaisesRegex(RuntimeError, "arguments for call are not valid"):
+        with self.assertRaisesRegex(RuntimeError, "Arguments for call are not valid"):
             @torch.jit.script
             def comp(l):
                 # type: (List[int]) -> List[float]
@@ -5599,14 +5599,14 @@ def test_manual_unwrap_opt(x):
                 x = torch.jit._unwrap_optional(x)
             return x  # noqa: T484
 
-        with self.assertRaisesRegex(RuntimeError, "arguments for call are not valid"):
+        with self.assertRaisesRegex(RuntimeError, "Arguments for call are not valid"):
             @torch.jit.script
             def or_error(x, y):
                 # type: (Optional[int], Optional[int]) -> None
                 if x is None or y is None:
                     print(x + y)  # noqa: T484
 
-        with self.assertRaisesRegex(RuntimeError, "arguments for call are not valid"):
+        with self.assertRaisesRegex(RuntimeError, "Arguments for call are not valid"):
             @torch.jit.script
             def and_error(x, y):
                 # type: (Optional[int], Optional[int]) -> None
@@ -5615,15 +5615,15 @@ def and_error(x, y):
                 else:
                     print(x + y)  # noqa: T484
 
-        with self.assertRaisesRegex(RuntimeError, "arguments for call are not valid"):
+        with self.assertRaisesRegex(RuntimeError, "Arguments for call are not valid"):
             @torch.jit.script
             def named_var(x):
                 # type: (Optional[int]) -> None
                 x_none = x is not None
                 if x_none:
                     print(x + 1)  # noqa: T484
 
-        with self.assertRaisesRegex(RuntimeError, "arguments for call are not valid"):
+        with self.assertRaisesRegex(RuntimeError, "Arguments for call are not valid"):
             @torch.jit.script
             def named_var_and(x, y):
                 # type: (Optional[int], Optional[int]) -> None
@@ -8551,7 +8551,7 @@ def fn(x : torch.Tensor, y : Tensor, z) -> Tuple[Tensor, Tensor, Tensor]:
                 f.write(code)
             fn = get_fn('test_type_annotation_py3', script_path)
 
-            with self.assertRaisesRegex(RuntimeError, r"expected a value of type 'Tensor' for argument"
+            with self.assertRaisesRegex(RuntimeError, r"Expected a value of type 'Tensor' for argument"
                                                       r" '0' but instead found type 'Tuple\[Tensor,"):
                 @torch.jit.script
                 def bad_fn(x):
@@ -8626,13 +8626,13 @@ def method(self, x):
                 y = self.baz(x)
                 return x
 
-        with self.assertRaisesRegex(RuntimeError, "expected at most 1 arguments but found 2"):
+        with self.assertRaisesRegex(RuntimeError, "Expected at most 1 arguments but found 2"):
             ModuleTooMany()
-        with self.assertRaisesRegex(RuntimeError, "argument 1 not provided"):
+        with self.assertRaisesRegex(RuntimeError, "Argument 1 not provided"):
             ModuleTooFew()
         with self.assertRaisesRegex(RuntimeError, "need 3 values .* found only 2"):
             ModuleTooManyAssign()
-        with self.assertRaisesRegex(RuntimeError, "argument 1 not provided."):
+        with self.assertRaisesRegex(RuntimeError, "Argument 1 not provided."):
             ModuleDefault()
 
     def test_script_define_order(self):
@@ -9462,12 +9462,12 @@ def foo(a, b):
 
     def test_builtin_args_fails(self):
 
-        with self.assertRaisesRegex(RuntimeError, 'expected at most'):
+        with self.assertRaisesRegex(RuntimeError, 'xpected at most'):
             @torch.jit.script
             def f0(a):
                 torch.sum(a, a, a, a)
 
-        with self.assertRaisesRegex(RuntimeError, 'argument self not provided'):
+        with self.assertRaisesRegex(RuntimeError, 'Argument self not provided'):
             @torch.jit.script
             def f1(a):
                 torch.sum(foo=4)
@@ -9833,7 +9833,7 @@ def forward(self, x):
             ReassignSelfRHS()
 
     def test_unknown_builtin(self):
-        with self.assertRaisesRegex(RuntimeError, 'unknown builtin op'):
+        with self.assertRaisesRegex(RuntimeError, 'Unknown builtin op'):
             @torch.jit.script
             def unknown_builtin(x):
                 return x.splork(3)
@@ -9916,7 +9916,7 @@ def multi_reduction(x):
             ''')
 
     def test_invalid_call_arguments(self):
-        with self.assertRaisesRegex(RuntimeError, 'arguments for call are not valid'):
+        with self.assertRaisesRegex(RuntimeError, 'Arguments for call are not valid'):
             @torch.jit.script
             def invalid_call_arguments(x):
                 return torch.unsqueeze(3, 4, 5, 6, 7, 8)
@@ -10000,7 +10000,7 @@ def wrong_module_attr_lookup():
                 return io.BytesIO
 
     def test_wrong_method_call_inputs(self):
-        with self.assertRaisesRegex(RuntimeError, 'argument y not provided'):
+        with self.assertRaisesRegex(RuntimeError, 'Argument y not provided'):
             class SomeModule(torch.jit.ScriptModule):
 
                 @torch.jit.script_method
@@ -10033,7 +10033,7 @@ def test():
             ''')
 
     def test_call_ge(self):
-        with self.assertRaisesRegex(RuntimeError, 'expected at most 1 arguments but found 3'):
+        with self.assertRaisesRegex(RuntimeError, 'Expected at most 1 arguments but found 3'):
             @_trace(torch.zeros(1, 2, 3))
             def foo(x):
                 return x
@@ -10824,7 +10824,7 @@ def return_tup(x):
                 return x, x  # noqa: T484
 
     def test_annotated_script_fn_arg_mismatch(self):
-        with self.assertRaisesRegex(RuntimeError, r"arguments for call are not valid"):
+        with self.assertRaisesRegex(RuntimeError, r"Arguments for call are not valid"):
             @torch.jit.script
             def tuple_arg(x):
                 # type: (Tuple[Tensor, Tensor]) -> Tensor
@@ -11318,7 +11318,7 @@ def forward(self, x):
             foo(torch.ones([123]))  # wrong size
 
     def test_builtin_error_messsage(self):
-        with self.assertRaisesRegex(RuntimeError, "arguments for call are not valid"):
+        with self.assertRaisesRegex(RuntimeError, "Arguments for call are not valid"):
             @torch.jit.script
             def close_match(x):
                 return x.masked_fill(True)
@@ -15644,7 +15644,7 @@ def set_non_initialized(self, y):
                     self.bar = y  # can't assign to non-initialized attr
 
     def test_type_annotations(self):
-        with self.assertRaisesRegex(RuntimeError, "expected a value of type \'bool"):
+        with self.assertRaisesRegex(RuntimeError, "Expected a value of type \'bool"):
             @torch.jit.script  # noqa: B903
             class FooTest(object):
                 def __init__(self, x):
diff --git a/torch/csrc/jit/script/schema_matching.cpp b/torch/csrc/jit/script/schema_matching.cpp
@@ -149,8 +149,7 @@ static Value* tryMatchArgument(
     if (failure_messages) {
       err() << "Could not match type " << value->type()->python_str() << " to "
             << arg.type()->python_str() << " in argument '" << arg.name()
-            << "': " << matched_type.errMsg << "\n"
-            << named_value.locOr(loc);
+            << "': " << matched_type.errMsg << ".\n";
     }
     return nullptr;
   }
@@ -169,16 +168,15 @@ static Value* tryMatchArgument(
         if (v->getElementType()->isSubtypeOf(TensorType::get())) {
           ostream << "Empty lists default to List[Tensor]. Use torch.jit."
                      "annotate(List[my_type], []) to create an empty list of"
-                     " another type\n";
+                     " another type.\n";
         }
       }
 
       if (value->type() == NumberType::get() &&
           value->node()->kind() == aten::item) {
         ostream << "Use int(tensor) or float(tensor) to retrieve item() from a "
-                << "tensor with the appropriate type\n";
+                << "tensor with the appropriate type.\n";
       }
-      ostream << named_value.locOr(loc);
     }
 
     return nullptr;
@@ -263,7 +261,7 @@ c10::optional<MatchedSchema> tryMatchSchema(
     std::ostream* failure_messages,
     bool allow_conversions) {
   auto err = [&]() -> std::ostream& {
-    *failure_messages << "\nfor operator " << schema << ":\n";
+    *failure_messages << "\n" << schema << ":\n";
     return *failure_messages;
   };
 
@@ -320,9 +318,8 @@ c10::optional<MatchedSchema> tryMatchSchema(
       const NamedValue& nv = kwargs[*kwarg_idx];
       if (used_kwarg[*kwarg_idx]) {
         if (failure_messages) {
-          err() << "argument " << nv.name()
-                << " specified twice in schema, submit a bug report!\n"
-                << nv.locOr(loc);
+          err() << "Argument " << nv.name()
+                << " specified twice in schema, submit a bug report!\n";
         }
         return c10::nullopt;
       }
@@ -334,9 +331,8 @@ c10::optional<MatchedSchema> tryMatchSchema(
       actual_named_value = NamedValue(*arg.default_value());
     } else {
       if (failure_messages) {
-        err() << "argument " << schema.arguments()[schema_i].name()
-              << " not provided.\n"
-              << loc;
+        err() << "Argument " << schema.arguments()[schema_i].name()
+              << " not provided.\n";
       }
       return c10::nullopt;
     }
@@ -358,7 +354,7 @@ c10::optional<MatchedSchema> tryMatchSchema(
   }
   // check for unused self argument
   if (self != c10::nullopt && failure_messages) {
-    err() << "provided self argument not used in schema\n";
+    err() << "Provided self argument not used in schema.\n";
   }
 
   if (schema.is_vararg()) {
@@ -370,9 +366,8 @@ c10::optional<MatchedSchema> tryMatchSchema(
   // check for unused positional arguments
   if (used_args < args.size()) {
     if (failure_messages) {
-      err() << "expected at most " << used_args << " arguments "
-            << "but found " << args.size() << " positional arguments.\n"
-            << loc << "\n";
+      err() << "Expected at most " << used_args << " arguments "
+            << "but found " << args.size() << " positional arguments.\n";
     }
     return c10::nullopt;
   }
@@ -382,9 +377,9 @@ c10::optional<MatchedSchema> tryMatchSchema(
     if (!used_kwarg[i]) {
       if (failure_messages) {
         if (!schema.argumentIndexWithName(nv.name())) {
-          err() << "keyword argument " << nv.name() << " unknown\n";
+          err() << "Keyword argument " << nv.name() << " unknown.\n";
         } else {
-          err() << "keyword argument " << nv.name() << " specified twice\n";
+          err() << "Keyword argument " << nv.name() << " specified twice.\n";
         }
       }
       return c10::nullopt;
@@ -557,23 +552,25 @@ Value* emitBuiltinCall(
     const auto close_symbols = findSimilarOperators(name);
     auto error = ErrorReport(loc);
     const auto& user_function_name = name.toQualString();
-    error << "unknown builtin op: " << user_function_name << "\n";
+    error << "Unknown builtin op: " << user_function_name << ".\n";
     if (close_symbols.size() == 0) {
       error
           << "Could not find any similar ops to " << user_function_name
-          << ". This op may not exist or may not be currently supported in TorchScript\n";
+          << ". This op may not exist or may not be currently supported in TorchScript.\n";
     } else {
       error << "Here are some suggestions: \n";
       for (const auto& sym : close_symbols) {
         error << "\t" << sym.toQualString() << "\n";
       }
+      error << "\nThe original call is";
     }
     throw error;
   }
 
-  throw ErrorReport(loc) << "arguments for call are not valid:\n"
+  throw ErrorReport(loc) << "Arguments for call are not valid.\n"
+                         << "The following operator variants are available:\n"
                          << prefixLine(failure_messages.str(), "  ")
-                         << "for call at";
+                         << "\nThe original call is";
 }
 } // namespace script
 } // namespace jit

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ struct Argument {`
`64`	`64`	`"because it was not annotated with an explicit type.\n");`
`65`	`65`	`}`
`66`	`66`	`return c10::str(`
`67`		`- "expected a value of type '",`
	`67`	`+ "Expected a value of type '",`
`68`	`68`	`type()->python_str(),`
`69`	`69`	`"' for argument '",`
`70`	`70`	`name(),`