Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IR][fix] Save value info for initializers #1552

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

justinchuby
Copy link
Collaborator

Previously initializers are not included in the graph value_info because they are not easily accessible from the Graph object. Now what we store all the Values for initializers, we can serialize the value information into the graph.

Fix #1501

@justinchuby justinchuby added the topic: IR Intermediate representation label May 16, 2024
Copy link

codecov bot commented May 16, 2024

❌ 9 Tests Failed:

Tests completed Failed Passed Skipped
8152 9 8143 3644
View the top 3 failed tests by shortest run time
onnxscript.rewriter.onnxruntime.instance_to_group_normalization_test.ReplaceInstanceNormWithGroupNormTest test_instance_norm_with_non_broadcasted_bias_full_should_remain
Stack Traces | 0.002s run time
.../rewriter/onnxruntime/instance_to_group_normalization_test.py:239: in test_instance_norm_with_non_broadcasted_bias_full_should_remain
    count = instance_to_group_normalization.rules.apply_to_model(model)
onnxscript/rewriter/pattern.py:1378: in apply_to_model
    count = self._apply_to_graph_or_function(model, model.graph, verbose=verbose)
onnxscript/rewriter/pattern.py:1356: in _apply_to_graph_or_function
    delta = rule.try_rewrite(model, graph_or_function, node, verbose=verbose)
onnxscript/rewriter/pattern.py:1230: in try_rewrite
    if not self._condition_function(context, **match.bindings):
.../rewriter/onnxruntime/instance_to_group_normalization.py:64: in _simulated_instance_norm
    weight_full_rank = len(weight_full.shape)
E   TypeError: object of type 'NoneType' has no len()
onnxscript.rewriter.onnxruntime.instance_to_group_normalization_test.ReplaceInstanceNormWithGroupNormTest test_instance_norm_with_rank_not_4_should_remain
Stack Traces | 0.002s run time
.../rewriter/onnxruntime/instance_to_group_normalization_test.py:276: in test_instance_norm_with_rank_not_4_should_remain
    count = instance_to_group_normalization.rules.apply_to_model(model)
onnxscript/rewriter/pattern.py:1378: in apply_to_model
    count = self._apply_to_graph_or_function(model, model.graph, verbose=verbose)
onnxscript/rewriter/pattern.py:1356: in _apply_to_graph_or_function
    delta = rule.try_rewrite(model, graph_or_function, node, verbose=verbose)
onnxscript/rewriter/pattern.py:1230: in try_rewrite
    if not self._condition_function(context, **match.bindings):
.../rewriter/onnxruntime/instance_to_group_normalization.py:64: in _simulated_instance_norm
    weight_full_rank = len(weight_full.shape)
E   TypeError: object of type 'NoneType' has no len()
onnxscript.rewriter.onnxruntime.instance_to_group_normalization_test.ReplaceInstanceNormWithGroupNormTest test_instance_norm_with_weight_full_having_multiple_not_one_dim_should_remain
Stack Traces | 0.002s run time
.../rewriter/onnxruntime/instance_to_group_normalization_test.py:315: in test_instance_norm_with_weight_full_having_multiple_not_one_dim_should_remain
    count = instance_to_group_normalization.rules.apply_to_model(model)
onnxscript/rewriter/pattern.py:1378: in apply_to_model
    count = self._apply_to_graph_or_function(model, model.graph, verbose=verbose)
onnxscript/rewriter/pattern.py:1356: in _apply_to_graph_or_function
    delta = rule.try_rewrite(model, graph_or_function, node, verbose=verbose)
onnxscript/rewriter/pattern.py:1230: in try_rewrite
    if not self._condition_function(context, **match.bindings):
.../rewriter/onnxruntime/instance_to_group_normalization.py:64: in _simulated_instance_norm
    weight_full_rank = len(weight_full.shape)
E   TypeError: object of type 'NoneType' has no len()

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

Copy link

github-actions bot commented May 16, 2024

Test Results

     24 files  ±      0      24 suites  ±0   42m 4s ⏱️ - 2h 46m 54s
 11 859 tests  -   3 830   7 857 ✅  -  5 827   3 989 💤 +  2 012   13 ❌  - 15 
101 688 runs   - 362 142  43 228 ✅  - 54 264  58 267 💤  - 307 842  193 ❌  - 36 

For more details on these failures, see this check.

Results for commit 64e740a. ± Comparison against base commit 9bae2b5.

This pull request removes 9868 and adds 6038 tests. Note that renamed tests count towards both.
docs.test.test_documentation_examples.TestDocumentationExample ‑ test_documentation_examples
tests.eager_mode_test.EagerModeTest_0_reference_runtime ‑ test_sequence_input
tests.eager_mode_test.EagerModeTest_1_onnxruntime ‑ test_sequence_input
tests.eager_mode_test.TestEagerModeArguments_0_reference_runtime ‑ test_function_all_input_by_kwargs
tests.eager_mode_test.TestEagerModeArguments_0_reference_runtime ‑ test_function_attribute_by_positional_args
tests.eager_mode_test.TestEagerModeArguments_0_reference_runtime ‑ test_function_input_and_attribute_by_kwargs_out_of_order
tests.eager_mode_test.TestEagerModeArguments_0_reference_runtime ‑ test_function_some_input_by_kwargs
tests.eager_mode_test.TestEagerModeArguments_0_reference_runtime ‑ test_op_all_input_by_kwargs
tests.eager_mode_test.TestEagerModeArguments_0_reference_runtime ‑ test_op_attribute_by_positional_args
tests.eager_mode_test.TestEagerModeArguments_0_reference_runtime ‑ test_op_input_and_attribute_by_kwargs_out_of_order
…
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0000_test_convinteger_without_padding
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0000_test_lrn_default
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0001_test_sce_NCd1d2d3_sum_weight_high_ii_expanded
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0001_test_sqrt
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0002_test_min_int32
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0002_test_nonmaxsuppression_suppress_by_IOU_and_scores
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0003_test_cast_no_saturate_FLOAT_to_FLOAT8E4M3FNUZ
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0003_test_col2im
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0004_test_reduce_l2_negative_axes_keep_dims_example
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0004_test_reduce_sum_square_do_not_keepdims_example_expanded
…
This pull request removes 14 skipped tests and adds 2018 skipped tests. Note that renamed tests count towards both.
tests.eager_test.TestOnnxSignal ‑ test_dft_rstft_04_A2
tests.eager_test.TestOnnxSignal ‑ test_dft_rstft_07_B2
tests.eager_test.TestOnnxSignal ‑ test_dft_rstft_10_C2
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__native_layer_norm_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__nn_functional_avg_pool1d_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__nn_functional_avg_pool2d_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__nn_functional_cross_entropy_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyFullGraphCPU ‑ test_output_match_opinfo__native_layer_norm_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyFullGraphCPU ‑ test_output_match_opinfo__nn_functional_avg_pool1d_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyFullGraphCPU ‑ test_output_match_opinfo__nn_functional_avg_pool2d_cpu_float16
…
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0003_test_cast_no_saturate_FLOAT_to_FLOAT8E4M3FNUZ
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0006_test_cast_FLOAT16_to_FLOAT8E4M3FN
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0007_test_sequence_insert_at_front
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0012_test_qlinearmatmul_2D_int8_float16
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0015_test_loop11
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0016_test_split_variable_parts_1d_opset13
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0017_test_if_seq
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0020_test_castlike_FLOAT_to_STRING
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0021_test_qlinearmatmul_2D_uint8_float32
onnxscript.backend.onnx_export_test.TestOnnxBackEnd ‑ test_export2python_produces_correct_onnx_script_model_0022_test_unsqueeze_two_axes
…
This pull request skips 16 and un-skips 8 tests.
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__div_mode_floor_rounding_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__floor_divide_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__index_put_bool_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__index_put_bool_cpu_float32
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__index_put_bool_cpu_int32
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__index_put_bool_cpu_int64
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__matmul_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__var_correction_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__var_dim_cpu_float16
tests.function_libs.torch_lib.ops_test.TestOutputConsistencyEagerCPU ‑ test_output_match_opinfo__var_mean_correction_cpu_float16
…
onnxscript.function_libs.torch_lib.graph_building.graph_building_test.TestModelSaving ‑ test_experimental_function_value_info_are_stored_in_graph_value_info
onnxscript.function_libs.torch_lib.graph_building.graph_building_test.TestModelSaving ‑ test_input_output_and_initializer_are_not_stored_in_value_info
onnxscript.function_libs.torch_lib.graph_building.graph_building_test.TestModelSaving ‑ test_save_initializer_to_files_for_large_model
onnxscript.ir.serde_test.TensorProtoTensorTest ‑ test_tensor_proto_tensor_bfloat16
onnxscript.tools.memory_peak_test.TestMemoryPeak ‑ test_memory
onnxscript.tools.memory_peak_test.TestMemoryPeak ‑ test_spy
onnxscript.tools.transformers_models.llama_test.TestExportLlama ‑ test_llama_dort_static
onnxscript.tools.transformers_models.phi_test.TestExportPhi ‑ test_phi_dort_static

♻️ This comment has been updated with latest results.

Copy link
Contributor

@titaiwangms titaiwangms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ci fails

if not initializer_name:
logger.warning(
"Initializer tensor must have a name but the %s-th initializer does not. Skipping this initializer.",
i,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we indicate it's index? Like f"initiliizer[{i}]"

@gramalingam
Copy link
Collaborator

Can this be closed, or is this still relevant?

@justinchuby
Copy link
Collaborator Author

This is still relavent. We need to update our test data first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: IR Intermediate representation
Projects
Development

Successfully merging this pull request may close these issues.

[IR] Values that are initialized but not graph inputs do not have their ValueInfo preserved
3 participants