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

[torchlib] Remove adaptive_avg_pool implementation #1751

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

justinchuby
Copy link
Collaborator

Remove adaptive_avg_pool implementation because our implementation using GlobalAveragePool is incorrect. We can rely on torch decomp instead.

@justinchuby justinchuby requested a review from titaiwangms July 23, 2024 23:44
@justinchuby justinchuby added the topic: torch_lib Related to the torch/aten function lib in development label Jul 23, 2024
Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.98%. Comparing base (874365e) to head (c94031a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1751      +/-   ##
==========================================
- Coverage   75.02%   74.98%   -0.04%     
==========================================
  Files         245      245              
  Lines       26417    26393      -24     
  Branches     4810     4804       -6     
==========================================
- Hits        19820    19792      -28     
- Misses       5677     5679       +2     
- Partials      920      922       +2     

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

Copy link

github-actions bot commented Jul 24, 2024

Test Results

     24 files  ±     0      24 suites  ±0   2h 58m 34s ⏱️ - 11m 27s
 15 566 tests  -  1 472  13 559 ✅  - 1 460    1 970 💤  -     16   37 ❌ +4 
423 670 runs   - 19 834  92 083 ✅  - 2 959  331 312 💤  - 16 878  275 ❌ +3 

For more details on these failures, see this check.

Results for commit c94031a. ± Comparison against base commit 874365e.

This pull request removes 1864 and adds 392 tests. Note that renamed tests count towards both.
onnxscript.function_libs.tools.torch_lib.deduce_type_constraints_test.TestDeduceTypeConstraints ‑ test_deduce_type_constraints_does_not_crash_for_onnx_function_aten_adaptive_avg_pool1d
onnxscript.function_libs.tools.torch_lib.deduce_type_constraints_test.TestDeduceTypeConstraints ‑ test_deduce_type_constraints_does_not_crash_for_onnx_function_aten_adaptive_avg_pool2d
onnxscript.function_libs.tools.torch_lib.deduce_type_constraints_test.TestDeduceTypeConstraints ‑ test_deduce_type_constraints_does_not_crash_for_onnx_function_aten_adaptive_avg_pool3d
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_168_aten_adaptive_avg_pool1d
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_169_aten_adaptive_avg_pool2d
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_170_aten_adaptive_avg_pool3d
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_171_aten_celu
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_172_aten_celu_type_promoted
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_173_aten_cross_entropy_loss
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_174_aten_dropout
…
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_168_aten_celu
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_169_aten_celu_type_promoted
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_170_aten_cross_entropy_loss
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_171_aten_dropout
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_172_aten_elu
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_173_aten_embedding_bag
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_174_aten_embedding_bag_padding_idx
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_175_aten_embedding_renorm
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_176_aten_embedding
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_function_has_op_schema_177_aten_hardsigmoid
…
This pull request removes 125 skipped tests and adds 109 skipped tests. Note that renamed tests count towards both.
onnxscript.function_libs.tools.torch_lib.deduce_type_constraints_test.TestDeduceTypeConstraints ‑ test_deduce_type_constraints_does_not_crash_for_onnx_function_aten_adaptive_avg_pool1d
onnxscript.function_libs.tools.torch_lib.deduce_type_constraints_test.TestDeduceTypeConstraints ‑ test_deduce_type_constraints_does_not_crash_for_onnx_function_aten_adaptive_avg_pool2d
onnxscript.function_libs.tools.torch_lib.deduce_type_constraints_test.TestDeduceTypeConstraints ‑ test_deduce_type_constraints_does_not_crash_for_onnx_function_aten_adaptive_avg_pool3d
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_176_aten_embedding_bag
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_177_aten_embedding_bag_padding_idx
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_203_aten_ones
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_204_aten_permute
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_207_aten_prelu
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_208_aten_rand
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_209_aten_rand_like
…
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_173_aten_embedding_bag
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_174_aten_embedding_bag_padding_idx
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_200_aten_ones
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_201_aten_permute
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_204_aten_prelu
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_205_aten_rand
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_206_aten_rand_like
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_207_aten_randint
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_208_aten_randint_low
tests.function_libs.torch_lib.ops_test.TestFunctionValidity ‑ test_script_function_passes_checker_209_aten_randint_like
…

♻️ 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.

What ops do we get after decomposition? I recall 1D is correct, but 2d and 3d is not passing test_fx_op_consistency.py

@justinchuby
Copy link
Collaborator Author

What ops do we get after decomposition? I recall 1D is correct, but 2d and 2d is not passing test_fx_op_consistency.py

Good point. I can test that. In any case globalavgpool is incorrect so we need to either improve the implementation or fix decomp

@justinchuby justinchuby merged commit c37e98b into main Jul 24, 2024
30 of 43 checks passed
@justinchuby justinchuby deleted the justinchu/remove-global-avg-pool branch July 24, 2024 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: torch_lib Related to the torch/aten function lib in development
Projects
Development

Successfully merging this pull request may close these issues.

2 participants