Checker should validate the Loop outputs have names.

[skottmckay]

Bug Report

Describe the bug

An ONNX Runtime user was attempting to create a Loop node where the loop carried dependencies were made optional outputs of the Loop node. i.e. they'd be passed around during loop iteration but not provided as an output of the Loop node. The spec doesn't allow the Loop outputs to be treated as optional, but the ONNX Checker does not fail the node.

System information

• OS Platform and Distribution (e.g. Linux Ubuntu 16.04): Windows 11
• ONNX version (e.g. 1.7): 1.10.2
• Python version: 3.9

Reproduction instructions

Below code will run the checker without it failing. It does have one hack to change the domain of the internal ONNX Runtime LayerNormalization operator which is incorrectly using the ai.onnx domain. This is so the body of the Loop is considered valid

import onnx

def change_domain(node, op_type, new_domain):
    if node.op_type == op_type:
        node.domain = new_domain

def iterate_graph_per_node_func(graph, per_node_func, **func_args):
    for node in graph.node:
        per_node_func(node, **func_args)
        # recurse into subgraph for control flow nodes (Scan/Loop/If)
        for attr in node.attribute:
            if attr.HasField('g'):
                iterate_graph_per_node_func(attr.g, per_node_func, **func_args)

model_path = "loop_toy_optional.onnx"

m = onnx.load(model_path)
m.opset_import.append(onnx.helper.make_operatorsetid('com.microsoft', 1))
iterate_graph_per_node_func(m.graph, change_domain, op_type='LayerNormalization', new_domain='com.microsoft')

onnx.checker.check_model(m)

• Attach the ONNX model to the issue (where applicable) See model in https://github.com/microsoft/onnxruntime/issues/10361

Expected behavior

Checker should require the Loop outputs to have valid names.

[jcwchen]

Reopen this issue to track again since we decided to revert the enhancement for checker by 4283 to unblock ONNX 1.12 release. Going forward, we should consider to make the check only for Loop op.