[pass] Remove unused initialized inputs in DCE

[leshabirukov]

Fix #2211

This pull request enhances the functionality of the RemoveUnusedNodesPass class and its associated methods by introducing an option to remove unused initialized inputs. It also updates the corresponding tests to validate this new behavior. The changes improve the flexibility of the unused node removal process and ensure the model input signature remains consistent unless explicitly modified.

Enhancements to RemoveUnusedNodesPass:

• Added a new remove_initialized_inputs attribute to the RemoveUnusedNodesPass class, allowing the removal of unused initialized inputs when enabled. This change modifies the model input signature if unused inputs are removed (onnxscript/ir/passes/common/unused_removal.py).

[justinchuby]

Looks good to me, thanks! Maybe create an option in the initializer so users can control?

[leshabirukov]

Looks good to me, thanks! Maybe create an option in the initializer so users can control?

May be. There are three alternatives:
do not remove inputs
remove all unused inputs
remove unused inputs-initializers

How about parameter remove_unused_inputs with default=False
True -> remove unused inputs-initializers

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.62%. Comparing base (feb20f1) to head (d12b6b2).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2212      +/-   ##
==========================================
+ Coverage   73.60%   73.62%   +0.02%     
==========================================
  Files         227      227              
  Lines       29052    29080      +28     
  Branches     3327     3330       +3     
==========================================
+ Hits        21383    21411      +28     
  Misses       6544     6544              
  Partials     1125     1125              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[justinchuby]

I think option three (remove unused inputs-initializers) is a good balance between correctness and user expectation. It sounds like a good idea to default it to True.

[justinchuby]

Maybe call the option remove_initialized_inputs?

[leshabirukov]

@microsoft-github-policy-service agree

[justinchuby]

https://github.com/onnx/onnx/blob/baae427e70c27d740f5bea859111e3b63d33512b/onnx/onnx.proto#L72C1-L74C25 mentions initializers does not have to follow inputs. I need to think about the default behavior a little more

[justinchuby]

onnx/onnx@baae427/onnx/onnx.proto#L72C1-L74C25 mentions initializers does not have to follow inputs. I need to think about the default behavior a little more

After thinking about this, I suggest to keep the same logic, but default the option to False.

[leshabirukov]

How I can approve it? It grayed

---

ok

[justinchuby]

Final round of suggestions. Thanks!

[leshabirukov]

aaand...

[justinchuby]

Thank you for making the contribution!

[Copilot]

Pull Request Overview

This PR enhances the unused node removal functionality by introducing a configurable option to remove unused initialized inputs. Key changes include:

• Updating the optimizer’s remove_unused_nodes function to accept a new remove_initialized_inputs parameter.
• Modifying tests in unused_removal_test.py to verify that unused initializer inputs are removed when requested.
• Updating the RemoveUnusedNodesPass to conditionally remove graph inputs matching unused initializers.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
onnxscript/optimizer/init.py	Modified call signature to pass remove_initialized_inputs parameter
onnxscript/ir/passes/common/unused_removal_test.py	Added tests to verify removal of unused initialized inputs
onnxscript/ir/passes/common/unused_removal.py	Updated pass implementation to remove corresponding graph inputs

Comments suppressed due to low confidence (1)

onnxscript/ir/passes/common/unused_removal.py:113

• [nitpick] Consider renaming the variable 'input' to avoid shadowing the built-in function 'input'.

for i, input in reversed(list(enumerate(graph_inputs))):

[justinchuby]

I think there is a format change required you can fix by running lintrunner f

[leshabirukov]

Errr...
What about this one?
=================================== FAILURES ===================================
_ TestOnnxBackEnd.test_export2python_produces_correct_onnx_script_model_0397_test_ai_onnx_ml_tree_ensemble_set_membership _
[gw0] linux -- Python 3.11.12 /home/runner/work/onnxscript/onnxscript/.nox/test_ort_nightly/bin/python
onnxscript/converter.py:460: in _eval_constant_expr
return eval(cpl, self.globals, locals) # pylint: disable=eval-used
E NameError: name 'nan' is not defined

The above exception was the direct cause of the following exception:
.nox/test_ort_nightly/lib/python3.11/site-packages/parameterized/parameterized.py:620: in standalone_func
return func(*(a + p.args), **p.kwargs, **kw)

[justinchuby]

Those checks are fine. They are unrelated.

[leshabirukov]

Hurray!