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

Migrate onnxrewriter #1346

Merged
merged 3 commits into from
Apr 5, 2024
Merged

Migrate onnxrewriter #1346

merged 3 commits into from
Apr 5, 2024

Conversation

Squashed of the following steps:
- #1328
- #1329
- #1330
- #1331
- #1332
- #1333
- #1343
- #1345

Co-authored-by: Shubham Bhokare <32080845+shubhambhokare1@users.noreply.github.com>
Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: Xavier Dupré <xadupre@users.noreply.github.com>
Co-authored-by: "G. Ramalingam" <grama@microsoft.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: Ti-Tai Wang <titaiwang@microsoft.com>

[ghstack-poisoned]
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

lintrunner found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

Attention: Patch coverage is 78.65643% with 1112 lines in your changes are missing coverage. Please review.

Project coverage is 64.83%. Comparing base (e29b43a) to head (596f3af).

Files Patch % Lines
onnxscript/optimizer/fold_constants_v0.py 0.00% 180 Missing ⚠️
...er/onnxruntime/transformers/multihead_attention.py 25.35% 159 Missing ⚠️
onnxscript/rewriter/generic_pattern.py 75.17% 100 Missing and 43 partials ⚠️
onnxscript/_legacy_ir/visitor.py 81.51% 83 Missing and 24 partials ⚠️
onnxscript/rewriter/generic_pattern_test.py 72.95% 63 Missing and 3 partials ⚠️
onnxscript/rewriter/pattern.py 87.89% 50 Missing and 15 partials ⚠️
onnxscript/testing/__init__.py 13.88% 62 Missing ⚠️
onnxscript/optimizer/evaluator.py 78.31% 22 Missing and 32 partials ⚠️
onnxscript/rewriter/function_rule.py 73.13% 26 Missing and 10 partials ⚠️
onnxscript/optimizer/copy_propagation.py 50.00% 26 Missing and 2 partials ⚠️
... and 39 more
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1346       +/-   ##
===========================================
- Coverage   76.05%   64.83%   -11.22%     
===========================================
  Files         134      140        +6     
  Lines       17782    19972     +2190     
  Branches     2935     3348      +413     
===========================================
- Hits        13524    12949      -575     
- Misses       3810     6480     +2670     
- Partials      448      543       +95     

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

Copy link

github-actions bot commented Apr 4, 2024

Test Results

    24 files  ±      0      24 suites  ±0   17m 48s ⏱️ - 1h 30m 16s
 8 181 tests  -   4 836   6 337 ✅  -  2 980  1 844 💤  -   1 855  0 ❌  - 1 
22 845 runs   - 253 183  18 402 ✅  - 44 885  4 443 💤  - 208 297  0 ❌  - 1 

Results for commit 667ab08. ± Comparison against base commit e29b43a.

♻️ This comment has been updated with latest results.

@titaiwangms titaiwangms added the change base before merge Remember to change the merge base to main when the PR is ready to merge label Apr 4, 2024
@BowenBao BowenBao changed the base branch from gh/BowenBao/32/base to main April 4, 2024 21:48
@titaiwangms titaiwangms removed the change base before merge Remember to change the merge base to main when the PR is ready to merge label Apr 5, 2024
Squashed of the following steps:
- #1328
- #1329
- #1330
- #1331
- #1332
- #1333
- #1343
- #1345

Co-authored-by: Shubham Bhokare <32080845+shubhambhokare1users.noreply.github.com>
Co-authored-by: Justin Chu <justinchubyusers.noreply.github.com>
Co-authored-by: Xavier Dupré <xadupreusers.noreply.github.com>
Co-authored-by: "G. Ramalingam" <gramamicrosoft.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnaviusers.noreply.github.com>
Co-authored-by: Ti-Tai Wang <titaiwangmicrosoft.com>

[ghstack-poisoned]
Comment on lines +1017 to +1020
# for name, init in pattern.initializers.items():
# # We add them to the graph, they will be removed if unused.
# new_name = g.make_initializer(name, init)
# replacements[new_name] = name

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
Comment on lines +601 to +603
# def exit_scope(self, graph: onnx.GraphProto) -> SubScope:
# # Also sync updated ir values back to value_info in graph.
# sub_scope = super().exit_scope(graph)

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
This is a placeholder, subject to simplification when a proper IR is defined.
"""

def get_input(self, node: onnx.NodeProto, index: int) -> ir.Value | None: ...

Check notice

Code scanning / CodeQL

Statement has no effect Note

This statement has no effect.

def get_input(self, node: onnx.NodeProto, index: int) -> ir.Value | None: ...

def get_output(self, node: onnx.NodeProto, index: int) -> ir.Value | None: ...

Check notice

Code scanning / CodeQL

Statement has no effect Note

This statement has no effect.

def get_output(self, node: onnx.NodeProto, index: int) -> ir.Value | None: ...

def input_const_value(self, node: onnx.NodeProto, index: int) -> ir.ConcreteValue: ...

Check notice

Code scanning / CodeQL

Statement has no effect Note

This statement has no effect.

def lookup_version(self, domain: str) -> int: ...

def convert_attributes(self, attributes: Sequence[onnx.AttributeProto]) -> dict: ...

Check notice

Code scanning / CodeQL

Statement has no effect Note

This statement has no effect.

def convert_attributes(self, attributes: Sequence[onnx.AttributeProto]) -> dict: ...

def new_constant(self, name: str, value: Any) -> Sequence[onnx.NodeProto] | None: ...

Check notice

Code scanning / CodeQL

Statement has no effect Note

This statement has no effect.
logger = logging.getLogger(__name__)


class FunctionRewriteError(RuntimeError): ...

Check notice

Code scanning / CodeQL

Statement has no effect Note

This statement has no effect.
# The rule is easy to create.


rule = org.make_pattern_rule(

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'rule' is unnecessary as it is
redefined
before this value is used.

# Currently, the replacement graph function is the same as the pattern function.
# This may change in the future.
_handle_replacement_return_value = _handle_pattern_return_value

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable '_handle_replacement_return_value' is not used.
Squashed of the following steps:
- #1328
- #1329
- #1330
- #1331
- #1332
- #1333
- #1343
- #1345

Co-authored-by: Shubham Bhokare <32080845+shubhambhokare1users.noreply.github.com>
Co-authored-by: Justin Chu <justinchubyusers.noreply.github.com>
Co-authored-by: Xavier Dupré <xadupreusers.noreply.github.com>
Co-authored-by: "G. Ramalingam" <gramamicrosoft.com>
Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnaviusers.noreply.github.com>
Co-authored-by: Ti-Tai Wang <titaiwangmicrosoft.com>

[ghstack-poisoned]
@BowenBao BowenBao merged commit 2c74be7 into main Apr 5, 2024
30 of 35 checks passed
@BowenBao BowenBao deleted the gh/BowenBao/32/head branch April 5, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants