Adding NodeDef names to error messages for better debuggability.

The format used is as follows:
{{node <node_name>}}

PiperOrigin-RevId: 206370355
This commit is contained in:
A. Unique TensorFlower 2018-07-27 14:03:25 -07:00 committed by TensorFlower Gardener
parent 388d0d8601
commit 81d927bfc7
17 changed files with 204 additions and 101 deletions

View File

@ -177,8 +177,8 @@ Status CheckNoCycleContains(const Node* node, const int num_nodes) {
visited[current_node->id()] = true;
for (const Edge* out : current_node->out_edges()) {
if (out->dst() == node) {
return errors::Internal("Detect a cycle: Node \"", node->name(), "\"(",
node->def().op(), ") feeds into itself.");
return errors::Internal("Detected a cycle: ", FormatNodeForError(*node),
"(", node->def().op(), ") feeds into itself.");
} else if (!visited[out->dst()->id()]) {
ready.push_back(out->dst());
}
@ -324,7 +324,7 @@ Status AddMissingFunctionDef(const FunctionDef& fdef,
if (library->Find(node.op())) {
continue;
}
// The function refered by 'SymbolicGradient' node is specified in its
// The function referred by 'SymbolicGradient' node is specified in its
// attribute 'f'.
if (node.op() == FunctionLibraryDefinition::kGradientOp) {
const AttrValue* attr =
@ -437,22 +437,24 @@ Status FunctionalizeLoop(const FunctionLibraryDefinition* lookup_library,
continue;
}
if (enter_merge != nullptr) {
return errors::Internal(
"Enter node for loop-varying argument ", arg.enter->name(),
" has multiple successors: ", enter_merge->dst()->name(), " and ",
e->dst()->name());
return errors::Internal("Enter node for loop-varying argument ",
FormatNodeForError(*arg.enter),
" has multiple successors: ",
FormatNodeForError(*enter_merge->dst()),
" and ", FormatNodeForError(*e->dst()));
}
enter_merge = e;
}
if (enter_merge == nullptr) {
return errors::Internal("Enter node for loop-varying argument ",
arg.enter->name(), " has zero successors");
FormatNodeForError(*arg.enter),
" has zero successors");
}
arg.merge = enter_merge->dst();
if (!IsMerge(arg.merge)) {
return errors::InvalidArgument(
"Successor of Enter node for loop-varying argument ",
arg.merge->name(),
FormatNodeForError(*arg.merge),
" is not a Merge node; got: ", arg.merge->type_string());
}
@ -462,7 +464,7 @@ Status FunctionalizeLoop(const FunctionLibraryDefinition* lookup_library,
return errors::InvalidArgument(
"Unexpected number of inputs to Merge node for loop-varying "
"argument ",
arg.merge->name(), "; expected 2, got ",
FormatNodeForError(*arg.merge), "; expected 2, got ",
arg.merge->input_types().size());
}
TF_RETURN_IF_ERROR(arg.merge->input_node(1 - enter_merge->dst_input(),
@ -470,7 +472,7 @@ Status FunctionalizeLoop(const FunctionLibraryDefinition* lookup_library,
if (!IsNextIteration(arg.next_iteration)) {
return errors::InvalidArgument(
"Expected NextIteration node as input to Merge node; got node ",
arg.next_iteration->name(), " with kind ",
FormatNodeForError(*arg.next_iteration), " with kind ",
arg.next_iteration->type_string());
}
@ -481,14 +483,14 @@ Status FunctionalizeLoop(const FunctionLibraryDefinition* lookup_library,
switches.find(edge->dst()) != switches.end()) {
if (arg.switch_node != nullptr) {
return errors::InvalidArgument("Duplicate Switch successors to ",
arg.merge->name());
FormatNodeForError(*arg.merge));
}
arg.switch_node = edge->dst();
}
}
if (arg.switch_node == nullptr) {
return errors::InvalidArgument("Missing Switch successor to ",
arg.merge->name());
FormatNodeForError(*arg.merge));
}
// Update the device on the Identity outputs of the switch to match their
@ -516,14 +518,15 @@ Status FunctionalizeLoop(const FunctionLibraryDefinition* lookup_library,
possible_exit.pop_front();
if (IsExit(edge->dst())) {
if (arg.exit != nullptr) {
return errors::InvalidArgument("Duplicate Exit successors to ",
arg.switch_node->name());
return errors::InvalidArgument(
"Duplicate Exit successors to ",
FormatNodeForError(*arg.switch_node));
}
arg.exit = edge->dst();
} else {
if (!IsIdentity(edge->dst())) {
return errors::Unimplemented("General graph between switch (",
arg.switch_node->name(),
FormatNodeForError(*arg.switch_node),
") and exit node of frame ",
frame->name, " not supported yet.");
}
@ -1470,7 +1473,7 @@ Status FunctionalizeControlFlow(const FunctionLibraryDefinition* lookup_library,
if (!unreachable_nodes.empty()) {
return errors::InvalidArgument(
"The following nodes are unreachable from the source in the graph: ",
tensorflow::str_util::Join(unreachable_nodes, ", "));
errors::FormatNodeNamesForError(unreachable_nodes));
}
// Builds Frames, indexed by name.

View File

@ -1064,7 +1064,10 @@ TEST(FunctionalizeControlFlow, Cycle) {
// less -> XlaIf <--> identity.
Status status = FunctionalizeControlFlow(graph.get(), &library);
EXPECT_FALSE(status.ok());
EXPECT_TRUE(str_util::StrContains(status.error_message(), "Detect a cycle"))
EXPECT_TRUE(str_util::StrContains(status.error_message(), "Detected a cycle"))
<< status.error_message();
EXPECT_TRUE(
str_util::StrContains(status.error_message(), "{{node cond/Less_5_If}}"))
<< status.error_message();
}

View File

@ -35,6 +35,7 @@ limitations under the License.
#include "tensorflow/core/common_runtime/function.h"
#include "tensorflow/core/common_runtime/graph_optimizer.h"
#include "tensorflow/core/framework/attr_value_util.h"
#include "tensorflow/core/framework/node_def_util.h"
#include "tensorflow/core/graph/algorithm.h"
#include "tensorflow/core/graph/graph_constructor.h"
#include "tensorflow/core/graph/node_builder.h"
@ -689,12 +690,12 @@ Status ValidateFunctionDef(const FunctionDef* fdef,
Status ValidateGraph(const Graph* graph,
const FunctionLibraryDefinition& flib_def,
const DeviceType& device_type, const string& name) {
auto maybe_error = [&](const string& op, const Status& s) -> Status {
auto maybe_error = [&](const Node* node, const Status& s) -> Status {
if (!s.ok()) {
return errors::InvalidArgument(strings::StrCat(
"Detected unsupported operations when trying to compile graph ", name,
" on ", device_type.type_string(), ": ", op, " (", s.error_message(),
")"));
" on ", device_type.type_string(), ": ", node->def().op(), " (",
s.error_message(), ")", FormatNodeForError(*node)));
}
return Status::OK();
};
@ -707,15 +708,15 @@ Status ValidateGraph(const Graph* graph,
Status s;
if (fdef) {
s = ValidateFunctionDef(fdef, flib_def);
TF_RETURN_IF_ERROR(maybe_error(node->def().op(), s));
TF_RETURN_IF_ERROR(maybe_error(node, s));
continue;
}
const OpDef* op_def;
s = OpRegistry::Global()->LookUpOpDef(node->def().op(), &op_def);
TF_RETURN_IF_ERROR(maybe_error(node->def().op(), s));
TF_RETURN_IF_ERROR(maybe_error(node, s));
TF_RETURN_IF_ERROR(ValidateNodeDef(node->def(), *op_def));
s = FindKernelDef(device_type, node->def(), nullptr, nullptr);
TF_RETURN_IF_ERROR(maybe_error(node->def().op(), s));
TF_RETURN_IF_ERROR(maybe_error(node, s));
}
return Status::OK();
}

View File

@ -312,7 +312,7 @@ TEST_F(XlaCompilerTest, HasSaneErrorOnNonCompileTimeConstantInputToReshape) {
str_util::StrContains(status.error_message(), "depends on a parameter"))
<< status.error_message();
EXPECT_TRUE(
str_util::StrContains(status.error_message(), "[[Node: C = Reshape"))
str_util::StrContains(status.error_message(), "[[{{node C}} = Reshape"))
<< status.error_message();
}
@ -1077,6 +1077,8 @@ TEST_F(XlaCompilerTest, FunctionWithInvalidOp) {
ASSERT_FALSE(status.ok());
EXPECT_TRUE(str_util::StrContains(status.error_message(), "InvalidOp"))
<< status.error_message();
EXPECT_TRUE(str_util::StrContains(status.error_message(), "{{node fill_fn}}"))
<< status.error_message();
}
// Tests a graph which has a node with invalid data type.
@ -1101,6 +1103,8 @@ TEST_F(XlaCompilerTest, NodeWithInvalidDataType) {
EXPECT_TRUE(str_util::StrContains(status.error_message(),
"is not in the list of allowed values"))
<< status.error_message();
EXPECT_TRUE(str_util::StrContains(status.error_message(), "{{node Shape}}"))
<< status.error_message();
}
TEST_F(XlaCompilerTest, SingleOpWithoutInputs) {
@ -1122,9 +1126,10 @@ TEST_F(XlaCompilerTest, SingleOpWithoutInputs) {
status = compiler.CompileGraph(XlaCompiler::CompileOptions(), "NoOp",
std::move(graph_copy), args, &result);
ASSERT_FALSE(status.ok());
EXPECT_TRUE(str_util::StrContains(status.error_message(),
"The following nodes are unreachable "
"from the source in the graph: NoOp"))
EXPECT_TRUE(
str_util::StrContains(status.error_message(),
"The following nodes are unreachable "
"from the source in the graph: {{node NoOp}}"))
<< status.error_message();
}

View File

@ -392,7 +392,7 @@
"output_type": "stream",
"text": [
"Got error message: assertion failed: [Do not pass zero!]\n",
"\t [[Node: f/Assert/Assert = Assert[T=[DT_STRING], summarize=3, _device=\"/job:localhost/replica:0/task:0/device:CPU:0\"](f/NotEqual, f/Assert/Assert/data_0)]]\n"
"\t [[{{node f/Assert/Assert}} = Assert[T=[DT_STRING], summarize=3, _device=\"/job:localhost/replica:0/task:0/device:CPU:0\"](f/NotEqual, f/Assert/Assert/data_0)]]\n"
]
}
],

View File

@ -1019,8 +1019,9 @@ TEST_F(FunctionLibraryRuntimeTest, Error_BadControlFlow) {
DCHECK_EQ(x.dtype(), DT_INT32);
Tensor y;
HasError(InstantiateAndRun(flr0_, "InvalidControlFlow", {}, {x}, {&y}),
"The node 'add' has inputs from different frames. The input 'enter' "
"is in frame 'while'. The input 'i' is in frame ''.");
"{{node add}} has inputs from different frames. The input"
" {{node enter}} is in frame 'while'. The input {{node i}} is in"
" frame ''.");
}
TEST_F(FunctionLibraryRuntimeTest, Gradient_XTimesTwo) {

View File

@ -86,7 +86,8 @@ string AttrSlice::SummarizeNode() const {
string SummarizeNode(const Node& node) { return SummarizeNodeDef(node.def()); }
string SummarizeNodeDef(const NodeDef& node_def) {
string ret = strings::StrCat(node_def.name(), " = ", node_def.op(), "[");
string ret = strings::StrCat(FormatNodeDefForError(node_def), " = ",
node_def.op(), "[");
strings::StrAppend(&ret, SummarizeAttrsHelper(node_def, node_def.device()));
strings::StrAppend(&ret, "](");
@ -101,6 +102,14 @@ string SummarizeNodeDef(const NodeDef& node_def) {
return ret;
}
string FormatNodeForError(const Node& node) {
return FormatNodeDefForError(node.def());
}
string FormatNodeDefForError(const NodeDef& node_def) {
return errors::FormatNodeNameForError(node_def.name());
}
const AttrValue* AttrSlice::Find(StringPiece attr_name) const {
// Currently, the collection used for NodeDef::attr() (google::protobuf::Map)
// requires that the keys used for lookups have type 'const string&'. Because
@ -634,7 +643,7 @@ Status ValidateExternalNodeDefSyntax(const NodeDef& node_def) {
Status AttachDef(const Status& status, const NodeDef& node_def) {
Status ret = status;
errors::AppendToMessage(
&ret, strings::StrCat(" [[Node: ", SummarizeNodeDef(node_def), "]]"));
&ret, strings::StrCat(" [[", SummarizeNodeDef(node_def), "]]"));
return ret;
}

View File

@ -50,6 +50,12 @@ extern const char* const kColocationGroupPrefix;
string SummarizeNode(const Node& node);
string SummarizeNodeDef(const NodeDef& node_def);
// Produces a formatted string pattern from the node which can uniquely identify
// this node upstream to produce an informative error message. The pattern
// followed is: {{node <node_name>}}
string FormatNodeForError(const Node& node);
string FormatNodeDefForError(const NodeDef& node_def);
typedef protobuf::Map<string, AttrValue> AttrValueMap;
// Adds an attr with name <name> and value <value> to *node_def.

View File

@ -20,6 +20,8 @@ limitations under the License.
#include "tensorflow/core/framework/node_def_builder.h"
#include "tensorflow/core/framework/op_def_builder.h"
#include "tensorflow/core/framework/op_def_util.h"
#include "tensorflow/core/graph/graph.h"
#include "tensorflow/core/graph/node_builder.h"
#include "tensorflow/core/lib/core/errors.h"
#include "tensorflow/core/lib/core/status_test_util.h"
#include "tensorflow/core/lib/strings/str_util.h"
@ -79,7 +81,7 @@ TEST(NodeDefUtilTest, In) {
)proto");
ExpectSuccess(node_def, op);
EXPECT_EQ("n = In[T=DT_FLOAT](a)", SummarizeNodeDef(node_def));
EXPECT_EQ("{{node n}} = In[T=DT_FLOAT](a)", SummarizeNodeDef(node_def));
// Mismatching Op names.
NodeDef bad = node_def;
@ -144,7 +146,7 @@ TEST(NodeDefUtilTest, Out) {
)proto");
ExpectSuccess(node_def, op);
EXPECT_EQ("n = Out[T=DT_INT32]()", SummarizeNodeDef(node_def));
EXPECT_EQ("{{node n}} = Out[T=DT_INT32]()", SummarizeNodeDef(node_def));
// Non-number type.
NodeDef bad = node_def;
@ -164,7 +166,7 @@ TEST(NodeDefUtilTest, Enum) {
)proto");
ExpectSuccess(node_def, op);
EXPECT_EQ("n = Enum[e=\"apple\"]()", SummarizeNodeDef(node_def));
EXPECT_EQ("{{node n}} = Enum[e=\"apple\"]()", SummarizeNodeDef(node_def));
NodeDef good = node_def;
good.clear_attr();
@ -191,7 +193,8 @@ TEST(NodeDefUtilTest, SameIn) {
)proto");
ExpectSuccess(node_def, op);
EXPECT_EQ("n = SameIn[N=2, T=DT_DOUBLE](a, b)", SummarizeNodeDef(node_def));
EXPECT_EQ("{{node n}} = SameIn[N=2, T=DT_DOUBLE](a, b)",
SummarizeNodeDef(node_def));
// Illegal type
NodeDef bad = ToNodeDef(R"proto(
@ -220,7 +223,7 @@ TEST(NodeDefUtilTest, AnyIn) {
)proto");
ExpectSuccess(node_def, op);
EXPECT_EQ("n = AnyIn[T=[DT_INT32, DT_STRING]](a, b)",
EXPECT_EQ("{{node n}} = AnyIn[T=[DT_INT32, DT_STRING]](a, b)",
SummarizeNodeDef(node_def));
const NodeDef bad = ToNodeDef(R"proto(
@ -243,13 +246,14 @@ TEST(NodeDefUtilTest, Device) {
const NodeDef node_def1 =
ToNodeDef(NodeDefBuilder("d", &op_def1).Device("/cpu:17"));
ExpectSuccess(node_def1, op_def1);
EXPECT_EQ("d = None[_device=\"/cpu:17\"]()", SummarizeNodeDef(node_def1));
EXPECT_EQ("{{node d}} = None[_device=\"/cpu:17\"]()",
SummarizeNodeDef(node_def1));
const OpDef op_def2 = ToOpDef(OpDefBuilder("WithAttr").Attr("v: int"));
const NodeDef node_def2 =
ToNodeDef(NodeDefBuilder("d", &op_def2).Attr("v", 7).Device("/cpu:5"));
ExpectSuccess(node_def2, op_def2);
EXPECT_EQ("d = WithAttr[v=7, _device=\"/cpu:5\"]()",
EXPECT_EQ("{{node d}} = WithAttr[v=7, _device=\"/cpu:5\"]()",
SummarizeNodeDef(node_def2));
}
@ -284,7 +288,7 @@ TEST(NodeDefUtilTest, ValidSyntax) {
)proto");
ExpectValidSyntax(node_def_explicit_inputs);
EXPECT_EQ("n = AnyIn[T=[DT_INT32, DT_STRING]](a:0, b:123)",
EXPECT_EQ("{{node n}} = AnyIn[T=[DT_INT32, DT_STRING]](a:0, b:123)",
SummarizeNodeDef(node_def_explicit_inputs));
const NodeDef node_def_partial_shape = ToNodeDef(R"proto(
@ -379,7 +383,7 @@ TEST(NameRangesForNodeTest, Simple) {
EXPECT_EQ(NameRangeMap({{"a", {0, 1}}, {"b", {1, 2}}}), inputs);
EXPECT_EQ(NameRangeMap({{"c", {0, 1}}, {"d", {1, 2}}}), outputs);
EXPECT_EQ("simple = Simple[](a, b)", SummarizeNodeDef(node_def));
EXPECT_EQ("{{node simple}} = Simple[](a, b)", SummarizeNodeDef(node_def));
OpDef bad_op_def = op_def;
bad_op_def.mutable_input_arg(0)->clear_type();
@ -399,7 +403,7 @@ TEST(NameRangesForNodeTest, Polymorphic) {
TF_EXPECT_OK(NameRangesForNode(node_def1, op_def, &inputs, &outputs));
EXPECT_EQ(NameRangeMap({{"a", {0, 1}}, {"b", {1, 2}}}), inputs);
EXPECT_EQ(NameRangeMap({{"c", {0, 1}}}), outputs);
EXPECT_EQ("poly = Polymorphic[T=DT_INT32](a, b)",
EXPECT_EQ("{{node poly}} = Polymorphic[T=DT_INT32](a, b)",
SummarizeNodeDef(node_def1));
const NodeDef node_def2 = ToNodeDef(NodeDefBuilder("poly", &op_def)
@ -408,7 +412,8 @@ TEST(NameRangesForNodeTest, Polymorphic) {
TF_EXPECT_OK(NameRangesForNode(node_def2, op_def, &inputs, &outputs));
EXPECT_EQ(NameRangeMap({{"a", {0, 1}}, {"b", {1, 2}}}), inputs);
EXPECT_EQ(NameRangeMap({{"c", {0, 1}}}), outputs);
EXPECT_EQ("poly = Polymorphic[T=DT_BOOL](a, b)", SummarizeNodeDef(node_def2));
EXPECT_EQ("{{node poly}} = Polymorphic[T=DT_BOOL](a, b)",
SummarizeNodeDef(node_def2));
}
TEST(NameRangesForNodeTest, NRepeats) {
@ -431,7 +436,8 @@ TEST(NameRangesForNodeTest, NRepeats) {
EXPECT_EQ(NameRangeMap({{"c", {0, 1}}, {"d", {1, 5}}, {"e", {5, 8}}}),
outputs);
EXPECT_EQ(
"nr = NRepeats[M=3, N=4, T=DT_FLOAT](a, a:1, a:2, a:3, b, b:1, b:2, b:3)",
"{{node nr}} = NRepeats[M=3, N=4, T=DT_FLOAT](a, a:1, a:2, a:3, b, b:1, "
"b:2, b:3)",
SummarizeNodeDef(node_def1));
const NodeDef node_def2 = ToNodeDef(NodeDefBuilder("nr", &op_def)
@ -442,7 +448,7 @@ TEST(NameRangesForNodeTest, NRepeats) {
EXPECT_EQ(NameRangeMap({{"a", {0, 2}}, {"b", {2, 4}}}), inputs);
EXPECT_EQ(NameRangeMap({{"c", {0, 1}}, {"d", {1, 3}}, {"e", {3, 10}}}),
outputs);
EXPECT_EQ("nr = NRepeats[M=7, N=2, T=DT_DOUBLE](a, a:1, b, b:1)",
EXPECT_EQ("{{node nr}} = NRepeats[M=7, N=2, T=DT_DOUBLE](a, a:1, b, b:1)",
SummarizeNodeDef(node_def2));
NodeDef bad_node_def = node_def2;
@ -471,7 +477,7 @@ TEST(NameRangesForNodeTest, TypeList) {
EXPECT_EQ(NameRangeMap({{"c", {0, 4}}, {"d", {4, 7}}, {"e", {7, 9}}}),
outputs);
EXPECT_EQ(
"tl = TypeList[T1=[DT_BOOL, DT_FLOAT],"
"{{node tl}} = TypeList[T1=[DT_BOOL, DT_FLOAT],"
" T2=[DT_FLOAT, DT_FLOAT, DT_FLOAT, DT_FLOAT],"
" T3=[DT_INT32, DT_DOUBLE, DT_STRING]](a, a:1, b, b:1, b:2, b:3)",
SummarizeNodeDef(node_def1));
@ -485,7 +491,8 @@ TEST(NameRangesForNodeTest, TypeList) {
EXPECT_EQ(NameRangeMap({{"c", {0, 1}}, {"d", {1, 3}}, {"e", {3, 10}}}),
outputs);
EXPECT_EQ(
"tl = TypeList[T1=[DT_INT32, DT_INT32, DT_INT32, DT_INT32, DT_INT32,"
"{{node tl}} = TypeList[T1=[DT_INT32, DT_INT32, DT_INT32, DT_INT32, "
"DT_INT32,"
" DT_INT32, DT_INT32], T2=[DT_DOUBLE], T3=[DT_DOUBLE, DT_STRING]]"
"(a, a:1, a:2, a:3, a:4, a:5, a:6, b)",
SummarizeNodeDef(node_def2));
@ -509,5 +516,20 @@ TEST(AddPrefixAndSuffixToNode, Enter) {
EXPECT_EQ("prefix/test_frame/suffix", frame_name);
}
TEST(FormatNodeForErrorTest, Node) {
Graph g(OpRegistry::Global());
Node* node;
TF_CHECK_OK(NodeBuilder("enter", "NoOp").Finalize(&g, &node));
EXPECT_EQ("{{node enter}}", FormatNodeForError(*node));
}
TEST(FormatNodeForErrorTest, NodeDef) {
NodeDef node_def;
node_def.set_name("enter");
node_def.set_op("Enter");
AddNodeAttr("frame_name", "test_frame", &node_def);
EXPECT_EQ("{{node enter}}", FormatNodeDefForError(node_def));
}
} // namespace
} // namespace tensorflow

View File

@ -209,8 +209,8 @@ TEST_F(OpCompatibilityTest, Same) {
.Finalize(node_def()));
ExpectSuccess(*RegisteredOpDef());
EXPECT_EQ(
"same = Same[N=3, T=DT_FLOAT, TList=[DT_BOOL, DT_BOOL]](a, b, c, c:1, "
"c:2, d, d:1, d:2, e, e:1)",
"{{node same}} = Same[N=3, T=DT_FLOAT, TList=[DT_BOOL, DT_BOOL]](a, b, "
"c, c:1, c:2, d, d:1, d:2, e, e:1)",
Result());
}
@ -224,7 +224,7 @@ TEST_F(OpCompatibilityTest, AddAttr) {
OpDefBuilder("AddAttr").Output("ndef: string").Finalize(&old_op));
TF_ASSERT_OK(NodeDefBuilder("add_attr", &old_op.op_def).Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("add_attr = AddAttr[a=42]()", Result());
EXPECT_EQ("{{node add_attr}} = AddAttr[a=42]()", Result());
}
// Should be able to make an attr restriction less strict.
@ -241,7 +241,7 @@ TEST_F(OpCompatibilityTest, LessStrict) {
.Attr("a", "B")
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("less_strict = LessStrict[a=\"B\"]()", Result());
EXPECT_EQ("{{node less_strict}} = LessStrict[a=\"B\"]()", Result());
}
// Should be able to remove an attr restriction.
@ -259,7 +259,8 @@ TEST_F(OpCompatibilityTest, RemoveRestriction) {
.Attr("a", DT_INT32)
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("remove_restriction = RemoveRestriction[a=DT_INT32]()", Result());
EXPECT_EQ("{{node remove_restriction}} = RemoveRestriction[a=DT_INT32]()",
Result());
}
// Should be able to change the order of attrs.
@ -278,7 +279,7 @@ TEST_F(OpCompatibilityTest, AttrOrder) {
.Attr("a", 7)
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("attr_order = AttrOrder[a=7, b=true]()", Result());
EXPECT_EQ("{{node attr_order}} = AttrOrder[a=7, b=true]()", Result());
}
// Should be able to make an input/output polymorphic.
@ -299,7 +300,8 @@ TEST_F(OpCompatibilityTest, TypePolymorphic) {
.Input(FakeInput())
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("type_polymorphic = TypePolymorphic[T=DT_INT32](a)", Result());
EXPECT_EQ("{{node type_polymorphic}} = TypePolymorphic[T=DT_INT32](a)",
Result());
}
// Should be able to make a single input/output into a list.
@ -320,7 +322,7 @@ TEST_F(OpCompatibilityTest, MakeList) {
.Input(FakeInput())
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("make_list = MakeList[N=1](a)", Result());
EXPECT_EQ("{{node make_list}} = MakeList[N=1](a)", Result());
}
// Should be able to make a single input/output into a polymorphic list.
@ -343,7 +345,8 @@ TEST_F(OpCompatibilityTest, MakePolyList) {
.Input(FakeInput())
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("make_poly_list = MakePolyList[N=1, T=DT_INT32](a)", Result());
EXPECT_EQ("{{node make_poly_list}} = MakePolyList[N=1, T=DT_INT32](a)",
Result());
}
// Should be able to make a single input/output into an arbitrary list.
@ -364,7 +367,7 @@ TEST_F(OpCompatibilityTest, MakeAnyList) {
.Input(FakeInput())
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("make_any_list = MakeAnyList[T=[DT_INT32]](a)", Result());
EXPECT_EQ("{{node make_any_list}} = MakeAnyList[T=[DT_INT32]](a)", Result());
}
// Should be able to make a single polymorphic input/output into a list of
@ -387,7 +390,8 @@ TEST_F(OpCompatibilityTest, PolyIntoList) {
.Input(FakeInput(DT_INT32))
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("poly_into_list = PolyIntoList[N=1, T=DT_INT32](a)", Result());
EXPECT_EQ("{{node poly_into_list}} = PolyIntoList[N=1, T=DT_INT32](a)",
Result());
}
// Should be able to make a multiple inputs/outputs into a list with
@ -413,7 +417,7 @@ TEST_F(OpCompatibilityTest, MakeMultipleSameList) {
.Input(FakeInput())
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("make_list = MakeMultipleSameList[N=2](a, b)", Result());
EXPECT_EQ("{{node make_list}} = MakeMultipleSameList[N=2](a, b)", Result());
}
// Changing from int32, float -> T
@ -437,8 +441,9 @@ TEST_F(OpCompatibilityTest, MakeMultipleAnyList) {
.Input(FakeInput())
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("make_list = MakeMultipleAnyList[T=[DT_INT32, DT_FLOAT]](a, b)",
Result());
EXPECT_EQ(
"{{node make_list}} = MakeMultipleAnyList[T=[DT_INT32, DT_FLOAT]](a, b)",
Result());
}
// Should be able to change the name of an input/output.
@ -455,7 +460,7 @@ TEST_F(OpCompatibilityTest, ChangeName) {
.Input(FakeInput())
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("change_name = ChangeName[](a)", Result());
EXPECT_EQ("{{node change_name}} = ChangeName[](a)", Result());
}
// Should be able to add an input/output of type
@ -473,7 +478,7 @@ TEST_F(OpCompatibilityTest, AddNInts) {
TF_ASSERT_OK(
NodeDefBuilder("add_n_ints", &old_op.op_def).Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("add_n_ints = AddNInts[N=0]()", Result());
EXPECT_EQ("{{node add_n_ints}} = AddNInts[N=0]()", Result());
}
// Should be able to add an input/output of type N * T
@ -492,7 +497,7 @@ TEST_F(OpCompatibilityTest, AddNSame) {
TF_ASSERT_OK(
NodeDefBuilder("add_n_same", &old_op.op_def).Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("add_n_same = AddNSame[N=0, T=DT_BOOL]()", Result());
EXPECT_EQ("{{node add_n_same}} = AddNSame[N=0, T=DT_BOOL]()", Result());
}
// Should be able to add an input/output of type N * T
@ -517,8 +522,10 @@ TEST_F(OpCompatibilityTest, AddNSameAsExisting) {
.Input(FakeInput(DT_STRING))
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("add_n_same_as_existing = AddNSameAsExisting[N=0, T=DT_STRING](a)",
Result());
EXPECT_EQ(
"{{node add_n_same_as_existing}} = AddNSameAsExisting[N=0, "
"T=DT_STRING](a)",
Result());
}
// Should be able to add an input/output of type T
@ -536,7 +543,7 @@ TEST_F(OpCompatibilityTest, AddAnyList) {
TF_ASSERT_OK(
NodeDefBuilder("add_any_list", &old_op.op_def).Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("add_any_list = AddAnyList[T=[]]()", Result());
EXPECT_EQ("{{node add_any_list}} = AddAnyList[T=[]]()", Result());
}
// Should be able to allow shorter lists.
@ -557,8 +564,10 @@ TEST_F(OpCompatibilityTest, ShorterAnyList) {
.Input(FakeInput(2, DT_BOOL))
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("shorter_any_list = ShorterAnyList[T=[DT_BOOL, DT_BOOL]](a, a:1)",
Result());
EXPECT_EQ(
"{{node shorter_any_list}} = ShorterAnyList[T=[DT_BOOL, DT_BOOL]](a, "
"a:1)",
Result());
}
REGISTER_OP("ShorterSameList")
@ -578,7 +587,8 @@ TEST_F(OpCompatibilityTest, ShorterSameList) {
.Input(FakeInput(2))
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("shorter_same_list = ShorterSameList[N=2](a, a:1)", Result());
EXPECT_EQ("{{node shorter_same_list}} = ShorterSameList[N=2](a, a:1)",
Result());
}
// Can remove a restriction to an attr
@ -597,7 +607,7 @@ TEST_F(OpCompatibilityTest, AttrRemoveRestriction) {
.Attr("t", DT_INT32)
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("remove_restriction = AttrRemoveRestriction[t=DT_INT32]()",
EXPECT_EQ("{{node remove_restriction}} = AttrRemoveRestriction[t=DT_INT32]()",
Result());
}
@ -619,7 +629,8 @@ TEST_F(OpCompatibilityTest, AttrLessRestrictive) {
.Attr("t", DT_INT32)
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("less_restrictive = AttrLessRestrictive[t=DT_INT32]()", Result());
EXPECT_EQ("{{node less_restrictive}} = AttrLessRestrictive[t=DT_INT32]()",
Result());
}
// Can remove a minimum from an attr.
@ -637,7 +648,7 @@ TEST_F(OpCompatibilityTest, AttrRemoveMin) {
.Attr("n", 4)
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("remove_min = AttrRemoveMin[n=4]()", Result());
EXPECT_EQ("{{node remove_min}} = AttrRemoveMin[n=4]()", Result());
}
// Can lower the minimum on an attr.
@ -655,7 +666,7 @@ TEST_F(OpCompatibilityTest, AttrLowerMin) {
.Attr("n", 4)
.Finalize(node_def()));
ExpectSuccess(old_op.op_def);
EXPECT_EQ("lower_min = AttrLowerMin[n=4]()", Result());
EXPECT_EQ("{{node lower_min}} = AttrLowerMin[n=4]()", Result());
}
// Can make a ref input into a non-ref input.

View File

@ -18,6 +18,7 @@ limitations under the License.
#include <deque>
#include <vector>
#include "tensorflow/core/framework/node_def_util.h"
#include "tensorflow/core/framework/types.h"
#include "tensorflow/core/graph/node_builder.h"
#include "tensorflow/core/lib/core/errors.h"
@ -54,10 +55,11 @@ Status ValidateControlFlowInfo(const Graph* graph,
frame.parent = parent;
frame.name = cf.frame_name;
} else if (frame.parent != parent) {
return errors::InvalidArgument(
return errors::Internal(
"Invalid loop structure: Mismatched parent frames for \"",
cf.frame_name, "\": \"", parent->name, "\" vs \"", frame.parent->name,
"\". This is an internal bug, please file a bug report with "
"\". The node giving this error: ", FormatNodeForError(*node),
"This is an internal bug, please file a bug report with "
"instructions on how to reproduce the error.");
}
if (IsLoopCond(node)) {
@ -69,9 +71,9 @@ Status ValidateControlFlowInfo(const Graph* graph,
!str_util::StrContains(node->name(), "LoopCounter")) {
return errors::InvalidArgument(
"Invalid loop structure: Loop \"", cf.frame_name,
"\" has more than one LoopCond node: \"", node->name(), "\" and \"",
frame.loop_cond->name(),
"\". This is an internal bug, please file a bug report with "
"\" has more than one LoopCond node: ", FormatNodeForError(*node),
" and ", FormatNodeForError(*frame.loop_cond),
". This is an internal bug, please file a bug report with "
"instructions on how to reproduce the error.");
}
frame.loop_cond = node;
@ -135,12 +137,11 @@ Status BuildControlFlowInfo(const Graph* g, std::vector<ControlFlowInfo>* info,
const string& parent_frame = (*info)[out_parent->id()].frame_name;
if (parent_frame != frame_name) {
return errors::InvalidArgument(
"The node '", out->name(),
"' has inputs from different "
"frames. The input '",
curr_node->name(), "' is in frame '", frame_name,
"'. The input '", parent_nodes[out->id()]->name(),
"' is in frame '", parent_frame, "'.");
FormatNodeForError(*out),
" has inputs from different frames. The input ",
FormatNodeForError(*curr_node), " is in frame '", frame_name,
"'. The input ", FormatNodeForError(*parent_nodes[out->id()]),
" is in frame '", parent_frame, "'.");
}
} else {
out_info->frame = out;
@ -148,7 +149,8 @@ Status BuildControlFlowInfo(const Graph* g, std::vector<ControlFlowInfo>* info,
TF_RETURN_IF_ERROR(
GetNodeAttr(out->attrs(), "frame_name", &out_info->frame_name));
if (out_info->frame_name.empty()) {
return errors::InvalidArgument("The Enter node ", out->name(),
return errors::InvalidArgument("The Enter ",
FormatNodeForError(*out),
" must have a frame name.");
}
}
@ -156,12 +158,11 @@ Status BuildControlFlowInfo(const Graph* g, std::vector<ControlFlowInfo>* info,
if (is_visited) {
if (out_info->frame_name != frame_name) {
return errors::InvalidArgument(
"The node '", out->name(),
"' has inputs from different "
"frames. The input '",
curr_node->name(), "' is in frame '", frame_name,
"'. The input '", parent_nodes[out->id()]->name(),
"' is in frame '", out_info->frame_name, "'.");
FormatNodeForError(*out),
" has inputs from different frames. The input ",
FormatNodeForError(*curr_node), " is in frame '", frame_name,
"'. The input ", FormatNodeForError(*parent_nodes[out->id()]),
" is in frame '", out_info->frame_name, "'.");
}
} else {
out_info->frame = frame;

View File

@ -63,6 +63,15 @@ TEST(ValidateControlFlowTest, InputsFromDifferentFrames) {
EXPECT_TRUE(str_util::StrContains(status.error_message(),
"has inputs from different frames"))
<< status.error_message();
EXPECT_TRUE(str_util::StrContains(status.error_message(),
"{{node outer/body/inner/Merge}}"))
<< status.error_message();
EXPECT_TRUE(str_util::StrContains(status.error_message(),
"{{node outer/body/inner/Enter}}"))
<< status.error_message();
EXPECT_TRUE(
str_util::StrContains(status.error_message(), "{{node outer/Switch}}"))
<< status.error_message();
}
TEST(ValidateControlFlowTest, MismatchedParentFrames) {
@ -102,6 +111,8 @@ TEST(ValidateControlFlowTest, MismatchedParentFrames) {
EXPECT_TRUE(
str_util::StrContains(status.error_message(), "Mismatched parent frames"))
<< status.error_message();
EXPECT_TRUE(str_util::StrContains(status.error_message(), "{{node Enter2}}"))
<< status.error_message();
}
TEST(ValidateControlFlowTest, TwoLoopCond) {
@ -125,6 +136,12 @@ TEST(ValidateControlFlowTest, TwoLoopCond) {
EXPECT_TRUE(str_util::StrContains(status.error_message(),
"more than one LoopCond node"))
<< status.error_message();
EXPECT_TRUE(
str_util::StrContains(status.error_message(), "{{node sub/LoopCond}}"))
<< status.error_message();
EXPECT_TRUE(
str_util::StrContains(status.error_message(), "{{node LoopCond}}"))
<< status.error_message();
}
} // namespace

View File

@ -19,6 +19,7 @@ limitations under the License.
#include <sstream>
#include "tensorflow/core/lib/core/status.h"
#include "tensorflow/core/lib/strings/str_util.h"
#include "tensorflow/core/lib/strings/strcat.h"
#include "tensorflow/core/platform/logging.h"
#include "tensorflow/core/platform/macros.h"
@ -118,6 +119,25 @@ DECLARE_ERROR(Unauthenticated, UNAUTHENTICATED)
#undef DECLARE_ERROR
// Produces a formatted string pattern from the name which can uniquely identify
// this node upstream to produce an informative error message. The pattern
// followed is: {{node <name>}}
// Note: The pattern below determines the regex _NODEDEF_NAME_RE in the file
// tensorflow/python/client/session.py
// LINT.IfChange
inline string FormatNodeNameForError(const string& name) {
return strings::StrCat("{{node ", name, "}}");
}
// LINT.ThenChange(//tensorflow/python/client/session.py)
template <typename T>
string FormatNodeNamesForError(const T& names) {
::tensorflow::str_util::Formatter<string> f(
[](string* output, const string& s) {
::tensorflow::strings::StrAppend(output, FormatNodeNameForError(s));
});
return ::tensorflow::str_util::Join(names, ", ", f);
}
// The CanonicalCode() for non-errors.
using ::tensorflow::error::OK;

View File

@ -85,7 +85,7 @@ TEST_F(EqualGraphDefTest, NoMatch) {
Input(e_.opts().WithName("A"));
Input(a_.opts().WithName("B"));
EXPECT_FALSE(Match());
EXPECT_EQ("Did not find expected node 'A = Input[]()'", diff_);
EXPECT_EQ("Did not find expected node '{{node A}} = Input[]()'", diff_);
}
TEST_F(EqualGraphDefTest, MissingNode) {
@ -93,7 +93,7 @@ TEST_F(EqualGraphDefTest, MissingNode) {
Input(e_.opts().WithName("B"));
Input(a_.opts().WithName("A"));
EXPECT_FALSE(Match());
EXPECT_EQ("Did not find expected node 'B = Input[]()'", diff_);
EXPECT_EQ("Did not find expected node '{{node B}} = Input[]()'", diff_);
}
TEST_F(EqualGraphDefTest, ExtraNode) {
@ -101,7 +101,7 @@ TEST_F(EqualGraphDefTest, ExtraNode) {
Input(a_.opts().WithName("A"));
Input(a_.opts().WithName("B"));
EXPECT_FALSE(Match());
EXPECT_EQ("Found unexpected node 'B = Input[]()'", diff_);
EXPECT_EQ("Found unexpected node '{{node B}} = Input[]()'", diff_);
}
TEST_F(EqualGraphDefTest, NodeOrder) {

View File

@ -143,7 +143,7 @@ If the device you have specified does not exist, you will get
```
InvalidArgumentError: Invalid argument: Cannot assign a device to node 'b':
Could not satisfy explicit device specification '/device:GPU:2'
[[Node: b = Const[dtype=DT_FLOAT, value=Tensor<type: float shape: [3,2]
[[{{node b}} = Const[dtype=DT_FLOAT, value=Tensor<type: float shape: [3,2]
values: 1 2 3...>, _device="/device:GPU:2"]()]]
```

View File

@ -1235,8 +1235,12 @@ class BaseSession(SessionInterface):
return _fetch_handler_run
# Captures the name of a node in an error status.
_NODEDEF_NAME_RE = re.compile(r'\[\[Node: ([^ ]*?) =')
# Captures the name of a node in an error status. The regex below matches
# both the old and the new formats:
# Old format: [[Node: <node_name> = ...]]
# New format: [[{{node <node_name>}} = ...]]
_NODEDEF_NAME_RE = re.compile(
r'\[\[(Node: )?(\{\{node )?([^\} ]*)(\}\})?\s*=')
def _do_run(self, handle, target_list, fetch_list, feed_dict, options,
run_metadata):
@ -1291,7 +1295,7 @@ class BaseSession(SessionInterface):
node_def = None
op = None
if m is not None:
node_name = m.group(1)
node_name = m.group(3)
try:
op = self._graph.get_operation_by_name(node_name)
node_def = op.node_def

View File

@ -73,7 +73,7 @@ class TestUtilTest(test_util.TensorFlowTestCase):
test_util.assert_equal_graph_def(def_57, def_75)
# Compare two unequal graphs
with self.assertRaisesRegexp(AssertionError,
r"^Found unexpected node 'seven"):
r"^Found unexpected node '{{node seven}}"):
test_util.assert_equal_graph_def(def_57, def_empty)
def testIsGoogleCudaEnabled(self):