Incompatibility with ONNX IR specification

[elfman2]

Hello,

In the ONNX IR Specification: message NodeProto { repeated string input = 1; // namespace Value repeated string output = 2; // namespace Value

// An optional identifier for this node in a graph. // This field MAY be absent in this version of the IR. optional string name = 3; // namespace Node

NeuralCasting use the name of Node as a dictionnary key. When Nodes do not contain any name, it is by default an empty string "" which collides for every no name Nodes:

def _create_op_nodes(graph : onnx.onnx_ml_pb2.GraphProto, nodes : list[Node]) -> [dict, dict]:
    CompilerLogger().info("Create op nodes")
    in_dict = {}
    out_dict = {}
    for op in graph.node:
        CompilerLogger().info("Create op node: " + op.name)

        name : str = op.name
        optype : str = op.op_type
        in_dict[name] = op.input
        out_dict[name] = op.output

An example of no name Node model which fails: https://github.com/onera/acetone/blob/main/tests/models/acas/acas_COC/nn_acas_COC.onnx

[alecerio]

Hi @elfman2

NeuralCasting is not compatible with all the onnx version, but just with the ones I needed for the use cases of my PhD. Try to export your model with the version of ONNX v14. It should work as all the ops of your model are supported. Let me know :)

[elfman2]

Hi @alecerio, ONNX v14 corresponds to tag v1.9.0 according to https://onnxruntime.ai/docs/reference/compatibility.html The https://github.com/onnx/onnx/blob/v1.9.0/onnx/onnx.proto indicates that Node name is optional:

// An optional identifier for this node in a graph.
  // This field MAY be absent in ths version of the IR.
  optional string name = 3;     // namespace Node

Therefore, it is not an export issue, but an implementation issue.

On my opinion, this field should not be optional. I'm part of a working group targeting a safety ONNX profile (SONNX). We could change this field to mandatory so that NeuralCasting would become compliant to SONNX. https://github.com/onnx/working-groups/tree/main/safety-related-profile

let me know if NeuralCasting is willing to become a SONNX code generator candidate.

[alecerio]

Hi @elfman2

I agree, if the name is optional, then it could be a problem for the code generation of the models including nodes without a name. I assumed that the name was an ID for an ONNX Node. Yes, I agree, that field should not be optional probably.

It would be a good match, we can talk about it. You can find my emails on neuralcasting repo, or you can text me on discord, my username is: alessandro_cerioli