Fix bug in handling of variadics in function schema creation (#15409)

### Description

The code handling variadic parameters when creating a schema for a
function has a minor bug.
The checking logic was nested inside a conditional, instead of being
outside.
Fix the logic, and add a test-case. This bugs manifests itself when the
first parameter in the
variadic list is not an input/output of the enclosing function.

### Motivation and Context

Fixes https://github.com/microsoft/onnxruntime/issues/15404

---------

Signed-off-by: Ganesan Ramalingam <grama@microsoft.com>
This commit is contained in:
G. Ramalingam 2023-04-12 14:32:24 -07:00 committed by GitHub
parent e1e8852213
commit e361e3f138
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 36 additions and 10 deletions

View file

@ -123,6 +123,12 @@ static void IOTypeConstraintHelper(const ONNX_NAMESPACE::FunctionProto& onnx_fun
const auto* node_op_schema = schema_registry->GetSchema(node.op_type(), domain_version, node.domain());
int variadic_arg_idx = -1;
for (int i = 0; i < node.input_size(); ++i) {
if (node_op_schema && variadic_arg_idx == -1) {
// The check is applied only if we have not seen a variadic parameter so far:
ORT_ENFORCE(static_cast<size_t>(i) < node_op_schema->inputs().size(),
"Too many inputs for op " + node.op_type());
}
auto& in_name = node.input().Get(i);
auto iter = input_name_idx_map.find(in_name);
if (iter != input_name_idx_map.end()) {
@ -157,12 +163,12 @@ static void IOTypeConstraintHelper(const ONNX_NAMESPACE::FunctionProto& onnx_fun
}
}
}
}
// if this is a variadic input there are no more inputs in the schema
if (node_op_schema && variadic_arg_idx == -1 &&
node_op_schema->inputs().at(schema_idx).GetOption() == OpSchema::FormalParameterOption::Variadic) {
variadic_arg_idx = i;
}
// if this is a variadic input there are no more inputs in the schema
if (node_op_schema && variadic_arg_idx == -1 &&
node_op_schema->inputs().at(i).GetOption() == OpSchema::FormalParameterOption::Variadic) {
variadic_arg_idx = i;
}
}
@ -202,12 +208,12 @@ static void IOTypeConstraintHelper(const ONNX_NAMESPACE::FunctionProto& onnx_fun
}
}
}
}
// if this is a variadic output there are no more outputs in the schema
if (node_op_schema && variadic_arg_idx == -1 &&
node_op_schema->outputs().at(schema_idx).GetOption() == OpSchema::FormalParameterOption::Variadic) {
variadic_arg_idx = i;
}
// if this is a variadic output there are no more outputs in the schema
if (node_op_schema && variadic_arg_idx == -1 &&
node_op_schema->outputs().at(i).GetOption() == OpSchema::FormalParameterOption::Variadic) {
variadic_arg_idx = i;
}
}

View file

@ -353,6 +353,26 @@ TEST(FunctionTest, Variadics) {
ASSERT_STATUS_OK(session_object.Initialize());
}
// A variation of the variadics issue above, where the first input/output of the
// variadic list is NOT an input/output of the function.
TEST(FunctionTest, VariadicsNonInputOutput) {
const char* code = R"(
<ir_version: 8, opset_import: ["" : 17, "local" : 1]>
mymodel (float[2] x) => (float[3] y) {
y = local.func (x)
}
<opset_import: ["" : 17 ], domain: "local">
func (a) => (y) {
b = Identity(a)
z = Concat <axis = 0> (b, a, b)
y, w = Split (z)
}
)";
Check(code, "x", {1.0, 2.0}, "y", {1.0, 2.0, 1.0});
}
// Test use of outer-scope names inside sub-graphs in functions that are inlined.
TEST(FunctionTest, OuterScopeName) {
const char* code = R"(