[WIP] Fix BatchNormalization layer export issue in ONNX model

[Copilot]

• Verify final name-fixing behavior and optimizer test conventions
• Add a focused regression test for unnamed optional outputs
• Remove the optimizer step that reassigns cleared output names
• Run targeted tests, lintrunner, secret scan, and security validation

• Fixes BatchNormalization layer with training_mode=1 results in incorrect ONNX model #2969

[Justin Chu (justinchuby)]

Copilot name fix pass is still needed. Let me know if onnx-ir needs updating or if there is another way to fix it

[codecov]

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.67%. Comparing base (c927620) to head (709d3c4).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
onnxscript/optimizer/_optimizer.py	94.11%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2988      +/-   ##
==========================================
+ Coverage   72.65%   72.67%   +0.01%     
==========================================
  Files         265      265              
  Lines       32260    32286      +26     
  Branches     3050     3056       +6     
==========================================
+ Hits        23438    23463      +25     
  Misses       7786     7786              
- Partials     1036     1037       +1     

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

[Bas Aarts (bas-aarts)]

Copilot, adding a call to RemoveUnusedNodesPass (the pass that started this issue to begin with) for the sole purpose of clearing the output names of unused inference BatchNormalization layers, is a heavy handed and lazy way to circumvent the issue.

Apologies for speaking out of turn Justin Chu (@justinchuby). I just had to.

[Justin Chu (justinchuby)]

Copilot please update as commented

[Copilot]

Copilot please update as commented

Updated in a7ec3f0. The second RemoveUnusedNodesPass is replaced by a BatchNormalization-only output-slot cleanup before NameFixPass; the regression now validates the serialized model with the ONNX checker. Current onnx-ir releases still need an upstream general fix.

[Bas Aarts (bas-aarts)]

Justin Chu (@justinchuby) , looks like it's time to move this over to onnx-ir.
These fixes by Copilot are getting worse on every try

[Justin Chu (justinchuby)]

I think ultimately we need to know how to deal with this op here. ONNX IR is agnostic to the opsets or operators.

[Bas Aarts (bas-aarts)]

while I would normally agree with you, in this case it very much feels like on onnx-ir short coming.
https://github.com/onnx/ir-py/blob/f0689c57f471ec7bd165661b43035b2353bacd18/src/onnx_ir/passes/common/unused_removal.py#L46-L49 clears the output names. This is done for BatchNormalization specifically, leaving unnamed outputs. A few lines below, there's code that explicitly removes such outputs https://github.com/onnx/ir-py/blob/f0689c57f471ec7bd165661b43035b2353bacd18/src/onnx_ir/passes/common/unused_removal.py#L67-L74, but this is skipped because of the early out https://github.com/onnx/ir-py/blob/f0689c57f471ec7bd165661b43035b2353bacd18/src/onnx_ir/passes/common/unused_removal.py#L51.
My guess is that simply removing that return would also fix this issue.

Fixing this in onnxscript feels a but too purist in my opinion, as it is clear that onnx-ir's intention is to remove such outputs