Remove function value error in version converter

[titaiwangms]

Fix #2790

This pull request makes a targeted change to the version converter in onnxscript. The main update removes the restriction that prevented models containing functions from being processed by the version conversion pass.

Version conversion support update:

• Removed the check that raised an error when the input model contained functions, allowing the version conversion pass to process such models without requiring prior inlining. (onnxscript/version_converter/__init__.py)

[codecov]

Codecov Report

❌ Patch coverage is 98.14815% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 70.50%. Comparing base (0206a98) to head (e251868).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
onnxscript/version_converter/_version_converter.py	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2791      +/-   ##
==========================================
+ Coverage   70.45%   70.50%   +0.05%     
==========================================
  Files         228      228              
  Lines       27177    27211      +34     
  Branches     2734     2737       +3     
==========================================
+ Hits        19148    19186      +38     
+ Misses       7092     7090       -2     
+ Partials      937      935       -2     

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

[titaiwangms]

TODO: support function within version converter

[Copilot]

Pull request overview

This pull request enables version conversion support for ONNX models containing functions, addressing issue #2790 where models with functions caused the version converter to fail after upstream changes in onnx-ir.

Changes:

• Removed the automatic inlining of functions before version conversion
• Added support for converting nodes inside function definitions directly
• Removed the error that prevented processing models with functions

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
onnxscript/version_converter/init.py	Removed InlinePass from the conversion pipeline and the ValueError check for models with functions
onnxscript/version_converter/_version_converter.py	Added visit_function method to handle version conversion of nodes inside functions and integrated it into the model visitor
onnxscript/version_converter/_version_converter_test.py	Added test case verifying that nodes inside model functions are correctly version-converted

[gramalingam]

Unfortunately, there is a problem with generalizing the version-converter to work for functions. The issues is that for nodes inside a Function, there may be attributes that are attribute-references ... and if the version-converter logic needs the values of these attributes, then they may not be able to handle nodes where there are attribute-references instead of actual attribute-values.

At the least, we need logic in the version-converter (for any op, where we check an attribute-value) to catch the case where it is an attribute-reference and raise an NotImplemented error. For example, see the logic here ... here we should not use the default value of zero if it is an attribute-reference; instead we should copy the attribute-reference. Some adjustment to the logic is required, along with test-cases.

[titaiwangms]

Unfortunately, there is a problem with generalizing the version-converter to work for functions. The issues is that for nodes inside a Function, there may be attributes that are attribute-references ... and if the version-converter logic needs the values of these attributes, then they may not be able to handle nodes where there are attribute-references instead of actual attribute-values.

At the least, we need logic in the version-converter (for any op, where we check an attribute-value) to catch the case where it is an attribute-reference and raise an NotImplemented error. For example, see the logic here ... here we should not use the default value of zero if it is an attribute-reference; instead we should copy the attribute-reference. Some adjustment to the logic is required, along with test-cases.

#2806