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] Improve aten::fill #1754

Merged
merged 2 commits into from
Jul 25, 2024
Merged

[torchlib] Improve aten::fill #1754

merged 2 commits into from
Jul 25, 2024

Conversation

justinchuby
Copy link
Collaborator

I updated torch-onnx to handle empty [] inputs, so the isinstance check is not needed.

@justinchuby justinchuby requested a review from titaiwangms July 25, 2024 17:42
Copy link

codecov bot commented Jul 25, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.96%. Comparing base (712aa87) to head (3243941).

Files Patch % Lines
onnxscript/tools/transformers_models/__init__.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1754      +/-   ##
==========================================
+ Coverage   74.95%   74.96%   +0.01%     
==========================================
  Files         245      245              
  Lines       26384    26383       -1     
  Branches     4803     4802       -1     
==========================================
+ Hits        19777    19779       +2     
+ Misses       5684     5682       -2     
+ Partials      923      922       -1     

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

@titaiwangms
Copy link
Contributor

Sounds like this might break torch.onnx.dynamo_export though?

Copy link

github-actions bot commented Jul 25, 2024

Test Results

     24 files  ±     0      24 suites  ±0   3h 6m 6s ⏱️ - 15m 54s
 15 565 tests + 2 179  13 563 ✅ +1 745    1 970 💤 +   432   32 ❌ +2 
441 516 runs   - 35 692  94 712 ✅  - 5 250  346 534 💤  - 30 444  270 ❌ +2 

For more details on these failures, see this check.

Results for commit 3243941. ± Comparison against base commit 712aa87.

♻️ This comment has been updated with latest results.

@justinchuby
Copy link
Collaborator Author

Sounds like this might break torch.onnx.dynamo_export though?

Probably not? Because it used to be like this

@titaiwangms
Copy link
Contributor

Sounds like this might break torch.onnx.dynamo_export though?

Probably not? Because it used to be like this

Oh you added it for torch-onnx.

@justinchuby justinchuby added the merge at lgtm Reviewers can merge when they approve label Jul 25, 2024
@justinchuby justinchuby merged commit 937558f into main Jul 25, 2024
28 of 43 checks passed
@justinchuby justinchuby deleted the justinchu/size branch July 25, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge at lgtm Reviewers can merge when they approve
Projects
Development

Successfully merging this pull request may close these issues.

2 participants