Skip to content

Commit

Permalink
Check graph outputs are defined (#6083)
Browse files Browse the repository at this point in the history
The current checker does not check that all graph outputs are defined.
This PR adds such a check. (See issue
#6040 ).

Note: We probably also want a separate check to ensure that a
graph-input is not used as a graph-output. But leaving that for a
separate PR, since it is a distinctly different check (and perhaps might
require a different discussion).

Redo of PR #6070

Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
  • Loading branch information
gramalingam committed Apr 12, 2024
1 parent 77127a7 commit 990cb66
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 3 deletions.
6 changes: 6 additions & 0 deletions onnx/checker.cc
Expand Up @@ -709,6 +709,12 @@ void check_graph(const GraphProto& graph, const CheckerContext& ctx, const Lexic
lex_ctx.add(output);
}
}
for (const auto& value_info : graph.output()) {
if (!lex_ctx.this_graph_has(value_info.name())) {
fail_check("Graph output '", value_info.name(), "' is not an output of any node in graph.");
}
}

print_warning_if_has_experimental(used_experimental_ops);
}

Expand Down
37 changes: 35 additions & 2 deletions onnx/test/checker_test.py
Expand Up @@ -797,7 +797,7 @@ def test_loop_with_different_initializer_input_below_ir4(self) -> None:
],
outputs=[
helper.make_tensor_value_info(
"cond___while_Identity_graph_outputs_Identity__3_0",
"cond___while_Less__13_0",
TensorProto.BOOL,
shape=[],
),
Expand Down Expand Up @@ -825,7 +825,7 @@ def test_loop_with_different_initializer_input_below_ir4(self) -> None:
outputs=["cond___while_Less__13_0"],
name="cond___while_Less__13",
domain="",
to=TensorProto.FLOAT,
to=TensorProto.BOOL,
),
],
),
Expand Down Expand Up @@ -1076,6 +1076,39 @@ def test_check_model_supports_unicode_path(self):
onnx.save(model, unicode_model_path)
checker.check_model(unicode_model_path, full_check=True)

def test_graph_output_is_defined(self):
model = onnx.parser.parse_model(
"""
<ir_version: 7, opset_import: [ "" : 17]>
agraph (float[N] x) => (float[N] y, float[N] z)
{
y = Add(x, x)
}
# Error: z is not defined
"""
)
self.assertRaises(checker.ValidationError, checker.check_model, model)

def test_graph_output_is_defined_within_sub_graph(self):
model = onnx.parser.parse_model(
"""
<ir_version: 7, opset_import: [ "" : 17]>
agraph (float[N] x, bool cond) => (float[N] y)
{
sum = Add (x, x)
prod = Mul (x, x)
y = If (cond) <
then_branch = then_graph () => (sum) {},
else_branch = else_graph () => (prod) {}
>
}
# Error: sum/prod are accessible inside if-then-else branches, but cannot
# be used as outputs of the then/else branch implicitly.
# An explicit "Identity(sum)" must be used to return sum as output.
"""
)
self.assertRaises(checker.ValidationError, checker.check_model, model)


if __name__ == "__main__":
unittest.main()
2 changes: 1 addition & 1 deletion onnx/test/compose_test.py
Expand Up @@ -87,7 +87,7 @@ def _make_sparse_tensor(name: str) -> SparseTensorProto:
{
C0 = Add(B01, B11)
C1 = Sub(B11, B21)
M1 = Mul(C0, C1)
D0 = Mul(C0, C1)
}
"""

Expand Down

0 comments on commit 990cb66

Please sign in to comment.