CleanUnusedInitializersAndNodeArgs warning seems to be redundant

[neNasko1]

Describe the issue

When a model with a constant in a subgraph is created, it generates the following warnings:

2024-01-15 06:38:44.474 test[87515:8919669] 2024-01-15 06:38:44.474063 [W:onnxruntime:, graph.cc:3572 CleanUnusedInitializersAndNodeArgs] Removing initializer 'If_0_else_branch__Constant_0_output'. It is not used by any node and should be removed from the model.
2024-01-15 06:38:44.474 test[87515:8919669] 2024-01-15 06:38:44.474718 [W:onnxruntime:, graph.cc:3572 CleanUnusedInitializersAndNodeArgs] Removing initializer 'If_0_then_branch__Constant_0_output'. It is not used by any node and should be removed from the model.

However, this current behavior doesn't seem to offer any value to the end-user. While there may be a reason for these warnings, the "problematic" nodes they refer to are removed in the same step.

This is demonstrated by the following Python code:

import onnxruntime

# Produces the warning
options = onnxruntime.SessionOptions()
options.optimized_model_filepath="model.onnx"
session = onnxruntime.InferenceSession('model.onnx', sess_options=options)

# Now model.onnx is the optimised model

# Doesn't produce the warning
options = onnxruntime.SessionOptions()
options.optimized_model_filepath="model.onnx"
session = onnxruntime.InferenceSession('model.onnx', sess_options=options)

The first session creation triggers the warning, but the second one, which uses the optimized model, does not. This suggests that the warning might not be necessary. Could we consider revising this behavior to improve the user experience?

To reproduce

Simple graph using spot:

import onnx
import numpy as np
from spox import argument, build, Tensor, Var
from spox.opset.ai.onnx import v17 as op
from spox.opset.ai.onnx.ml.v3 import label_encoder

a = argument(Tensor(np.int64, tuple()))
c = op.if_(
    op.equal(a, op.const(1))
    ,
    then_branch=lambda: [
        op.const([1])
    ],
    else_branch=lambda: [
        op.const([2])
    ]
)[0]

model: onnx.ModelProto = build(inputs={'a': a}, outputs={'c': c})
onnx.save(model, "model.onnx")

Urgency

No response

Platform

Mac

OS Version

14.1

ONNX Runtime Installation

Built from Source

ONNX Runtime Version or Commit ID

upstream

ONNX Runtime API

Python

Architecture

ARM64

Execution Provider

Default CPU

Execution Provider Library Version

No response

[skottmckay]

Until the model is updated to remove the unused initializers (by saving the model via ORT or running some other cleanup tool) they will waste time and memory during model loading. Depending on your scenario that may or may not be critical.

Without the warning the user would have no signal that they could benefit from removing these initializers.

[neNasko1]

Isn't that a fact that applies to every type of optimization. Do you recon that the warning should be more general and look something like: "Warning: optimisations were performed on the model, export it to benefit from the exported model".

[cbourjau]

@skottmckay The issue is not that anything could be removed, though. Onnxruntime replaces constants with initializers which appear to be faster to load, but that seems to be an implementation detail. IMHO, the warning message is misleading in the way that it is worded. Secondly, I think the importance of that information does not justify the warning level. If onnxruntime wants to inform users about potential optimizations of an otherwise perfectly valid graph, I believe an info rather than a warning should be logged.

[neNasko1]

It's crucial to highlight that, despite the occasional relevance of this warning, it is currently associated with a bug:

The check in question presents an issue as it verifies if we are in the initial Graph::Resolve. However, if graph->GraphResolveNeeded() returns false for a specific graph during initialization, then num_resolves_ will not increment prior to executing optimizations. Consequently, this means that each constant returned from a subgraph will trigger the aforementioned warning.

#19143 tries to resolve this by running CleanUnusedInitializersAndNodeArgs on all subgraphs in the beginning of the graph transformations.

[neNasko1]

@skottmckay I have made the PR ready for review. Since I was not clear enough with the examples I will provide additional examples where the warning shows up unexpectedly.

test_model.zip

In no_warning.onnx we expect no warnings to show as there are no unused initialisers. In warning.onnx we expect 1 warning to show up for the unused initialiser Initializer_0_arg_.

Upstream we get:

(onnxruntime) ➜  onnxruntime git:(main) ✗ ./test test_models/no_warning.onnx 
2024-01-22 17:55:01.854 test[47988:2586945] 2024-01-22 17:55:01.853871 [W:onnxruntime:, graph.cc:3572 CleanUnusedInitializersAndNodeArgs] Removing initializer 'If_0_else_branch__Constant_0_output'. It is not used by any node and should be removed from the model.
2024-01-22 17:55:01.854 test[47988:2586945] 2024-01-22 17:55:01.854336 [W:onnxruntime:, graph.cc:3572 CleanUnusedInitializersAndNodeArgs] Removing initializer 'If_0_then_branch__Constant_0_output'. It is not used by any node and should be removed from the model.

(onnxruntime) ➜  onnxruntime git:(main) ✗ ./test test_models/warning.onnx 
2024-01-22 17:54:58.578 test[47972:2586784] 2024-01-22 17:54:58.578170 [W:onnxruntime:, graph.cc:3572 CleanUnusedInitializersAndNodeArgs] Removing initializer 'Initializer_0_arg_'. It is not used by any node and should be removed from the model.
2024-01-22 17:54:58.581 test[47972:2586784] 2024-01-22 17:54:58.581333 [W:onnxruntime:, graph.cc:3572 CleanUnusedInitializersAndNodeArgs] Removing initializer 'If_0_else_branch__Constant_0_output'. It is not used by any node and should be removed from the model.
2024-01-22 17:54:58.581 test[47972:2586784] 2024-01-22 17:54:58.581351 [W:onnxruntime:, graph.cc:3572 CleanUnusedInitializersAndNodeArgs] Removing initializer 'If_0_then_branch__Constant_0_output'. It is not used by any node and should be removed from the model.

After changes we get:

(onnxruntime) ➜  onnxruntime git:(remove-clean-warning) ✗ ./test test_models/warning.onnx                               
2024-01-22 18:07:32.015 test[52935:2602415] 2024-01-22 18:07:32.015070 [W:onnxruntime:, graph.cc:3575 CleanUnusedInitializersAndNodeArgs] Removing initializer 'Initializer_0_arg_'. It is not used by any node and should be removed from the model.

(onnxruntime) ➜  onnxruntime git:(remove-clean-warning) ✗ ./test test_models/no_warning.onnx                            
(onnxruntime) ➜  onnxruntime git:(remove-clean-warning) ✗

[neNasko1]

@skottmckay can you take a look as you are also the person that pops up in the gitblame.

[skottmckay]

I'm not aware of a path where we do not call Graph::Resolve after loading a model. It's required to populate operator schema information that the optimizers require.

The real problem is we're only incrementing num_resolves_ for the main graph, so the subgraph reporting is incorrect as it's always stuck at 0.

#19341 should address that.

[neNasko1]

Thank you, this simpler change seems to target the issue in the provided samples as well.