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

Implement aten::{all,any}.dims | feat(torchlib) #1084

Merged
merged 8 commits into from
Oct 13, 2023
Merged

Conversation

justinchuby
Copy link
Collaborator

@justinchuby justinchuby commented Oct 4, 2023

Implement aten::{all,any}.dims according to pytorch/pytorch#110310

Tests will be enabled after the PyTorch PR is merged

@justinchuby justinchuby added the topic: torch_lib Related to the torch/aten function lib in development label Oct 4, 2023
@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Merging #1084 (8eb0137) into main (a981b8a) will decrease coverage by 0.13%.
The diff coverage is 23.52%.

@@            Coverage Diff             @@
##             main    #1084      +/-   ##
==========================================
- Coverage   78.10%   77.98%   -0.13%     
==========================================
  Files         115      115              
  Lines       14826    14860      +34     
  Branches     1574     1580       +6     
==========================================
+ Hits        11580    11588       +8     
- Misses       2875     2901      +26     
  Partials      371      371              
Files Coverage Δ
onnxscript/function_libs/torch_lib/ops/core.py 79.45% <23.52%> (-0.65%) ⬇️

@github-actions
Copy link

github-actions bot commented Oct 4, 2023

Test Results

         18 files  ±  0         18 suites  ±0   1h 18m 35s ⏱️ + 5m 14s
  11 036 tests +  2    8 296 ✔️ +  2      2 723 💤 ±0       17 ±0 
155 511 runs  +18  35 546 ✔️ +18  118 516 💤 ±0  1 449 ±0 

For more details on these failures, see this check.

Results for commit 0ccd9ad. ± Comparison against base commit a981b8a.

♻️ This comment has been updated with latest results.

@justinchuby justinchuby requested a review from BowenBao October 4, 2023 17:56


@torch_op("aten::any.dims")
def aten_any_dims_empty_dim(self: TTensor, keepdims: bool) -> BOOL:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like we still need the place holder for it when it is optional

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

NVM. dims is an attribute. This is good.



@torch_op("aten::any.dims")
def aten_any_dims_empty_dim(self: TTensor, keepdims: bool) -> BOOL:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Change 'empty' to 'no' to avoid confusion that users might think dims is an empty list/tuple where it is actually None.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

dims can be an empty list or None. They will mean the same thing. I made the changes nevertheless.

Copy link
Contributor

@fatcat-z fatcat-z left a comment

Choose a reason for hiding this comment

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

Thanks!

onnxscript/function_libs/torch_lib/ops/core.py Outdated Show resolved Hide resolved
onnxscript/function_libs/torch_lib/ops/core.py Outdated Show resolved Hide resolved
onnxscript/function_libs/torch_lib/ops/core.py Outdated Show resolved Hide resolved
onnxscript/function_libs/torch_lib/ops/core.py Outdated Show resolved Hide resolved
@justinchuby justinchuby merged commit 2187973 into main Oct 13, 2023
25 of 29 checks passed
@justinchuby justinchuby deleted the justinchu/anyalldims branch October 13, 2023 01:09
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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants