Skip to content

[Migration][DO NOT MERGE] Separate old ir into _legacy_ir folder #1332

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 7 commits into from
Closed

[Migration][DO NOT MERGE] Separate old ir into _legacy_ir folder #1332

wants to merge 7 commits into from

Conversation

BowenBao
Copy link
Contributor

@BowenBao BowenBao commented Apr 1, 2024 •
edited
Loading

@BowenBao
[Migration][DO NOT MERGE] Separate old ir into _legacy_ir folder
3adb570 
All tests except for linter are expected to pass.

[ghstack-poisoned]
This was referenced Apr 1, 2024
Closed
Closed
Closed
Closed
BowenBao added a commit that referenced this pull request Apr 1, 2024
@BowenBao
[Migration][DO NOT MERGE] Separate old ir into _legacy_ir folder
9246665 
All tests except for linter are expected to pass.

ghstack-source-id: 313925c
Pull Request resolved: #1332
@codecov Codecov
Copy link

codecov bot commented Apr 2, 2024 •
edited
Loading

Codecov Report

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

Project coverage is 64.17%. Comparing base (290d21e) to head (d0b0c34).

Files Patch % Lines
onnxscript/_legacy_ir/__init__.py 91.66% 9 Missing and 4 partials ⚠️
Additional details and impacted files 
@@                   Coverage Diff                   @@
##           gh/BowenBao/24/base    #1332      +/-   ##
=======================================================
+ Coverage                64.16%   64.17%   +0.01%     
=======================================================
  Files                      135      136       +1     
  Lines                    19019    19019              
  Branches                  3120     3120              
=======================================================
+ Hits                     12204    12206       +2     
+ Misses                    6318     6316       -2     
  Partials                   497      497

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

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 2, 2024 •
edited
Loading

Test Results

    24 files  ±    0      24 suites  ±0   21m 24s ⏱️ + 1m 42s
 9 423 tests +1 250   7 367 ✅ +1 037  2 053 💤 +210  3 ❌ +3 
22 749 runs  ±    0  18 315 ✅  -     3  4 431 💤 ±  0  3 ❌ +3 

For more details on these failures, see this check.

Results for commit d0b0c34. ± Comparison against base commit 290d21e.

♻️ This comment has been updated with latest results.

github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 2, 2024
View reviewed changes
onnxscript/_legacy_ir/__init__.py
@@ -0,0 +1,297 @@
from __future__ import annotations

Check warning

Code scanning / lintrunner

RUFF/I001

Import block is un-sorted or un-formatted. See https://docs.astral.sh/ruff/rules/unsorted-imports
onnxscript/_legacy_ir/__init__.py
@@ -0,0 +1,297 @@
from __future__ import annotations

Check warning

Code scanning / lintrunner

RUFF/format

Run `lintrunner -a` to apply this patch.
@BowenBao
Fix tests affected by moving 'tests' folder on "[Migration][DO NOT ME…
c577bd1 
…RGE] Separate old ir into `_legacy_ir` folder"

All tests except for linter are expected to pass.

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Apr 2, 2024
@BowenBao
[Migration][DO NOT MERGE] Separate old ir into _legacy_ir folder
a8210ae 
All tests except for linter are expected to pass.

ghstack-source-id: d2dd84b
Pull Request resolved: #1332
@BowenBao
Update CI to git lfs pull on "[Migration][DO NOT MERGE] Separate old …
96114ce 
…ir into `_legacy_ir` folder"

All tests except for linter are expected to pass.

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Apr 2, 2024
@BowenBao
[Migration][DO NOT MERGE] Separate old ir into _legacy_ir folder
1ceeeca 
All tests except for linter are expected to pass.

ghstack-source-id: 3423e36
Pull Request resolved: #1332
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 2, 2024
View reviewed changes
onnxscript/_legacy_ir/__init__.py
return self.node

def def_index(self) -> int:
return self.output_index

Check failure

Code scanning / lintrunner

MYPY/return-value

Incompatible return value type (got "int | None", expected "int") To disable, use ` # type: ignore[return-value]`
onnxscript/_legacy_ir/__init__.py
shape = self.shape
if shape is not None:
shape = [str(dim) for dim in shape]
shape_str = f"[{', '.join(shape)}]"

Check failure

Code scanning / lintrunner

MYPY/arg-type

Argument 1 to "join" of "str" has incompatible type "list[str | int | None]"; expected "Iterable[str]" To disable, use ` # type: ignore[arg-type]`
onnxscript/_legacy_ir/__init__.py
class Function:
def __init__(self, function_proto: onnx.FunctionProto):
self.original_function_proto = function_proto
self.nodes = deque()

Check failure

Code scanning / lintrunner

MYPY/var-annotated

Need type annotation for "nodes" To disable, use ` # type: ignore[var-annotated]`
onnxscript/_legacy_ir/__init__.py
def __init__(self, function_proto: onnx.FunctionProto):
self.original_function_proto = function_proto
self.nodes = deque()
self.values = {}

Check failure

Code scanning / lintrunner

MYPY/var-annotated

Need type annotation for "values" (hint: "values: Dict[<type>, <type>] = ...") To disable, use ` # type: ignore[var-annotated]`
onnxscript/_legacy_ir/__init__.py
self.attributes: dict[str, int | float | RefAttr | Graph | list[Graph]] = {}

def get_attribute(self, name: str) -> int | float | None:
return self.attributes.get(name, None)

Check failure

Code scanning / lintrunner

MYPY/return-value

Incompatible return value type (got "int | float | RefAttr | Graph | list[Graph] | None", expected "int | float | None") To disable, use ` # type: ignore[return-value]`
@justinchuby
Copy link
Collaborator

Be sure to remove reference from ir/init.py as well?

@BowenBao
Fix legacy ir stuff on "[Migration][DO NOT MERGE] Separate old ir int…
930df9e 
…o `_legacy_ir` folder"

All tests except for linter are expected to pass.

[ghstack-poisoned]
@BowenBao BowenBao mentioned this pull request Apr 2, 2024
Closed
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 2, 2024
View reviewed changes
onnxscript/rewriter/function_rule.py
@@ -7,8 +7,8 @@
import onnxscript
from packaging import version

from onnxscript import ir
from onnxscript.ir import visitor
import onnxscript._legacy_ir as ir

Check notice

Code scanning / lintrunner

PYLINT/C0412

Imports from package onnxscript are not grouped (ungrouped-imports) See [ungrouped-imports](https://pylint.pycqa.org/en/latest/user_guide/messages/convention/ungrouped-imports.html). To disable, use ` # pylint: disable=ungrouped-imports`
@BowenBao BowenBao mentioned this pull request Apr 2, 2024
Closed
@BowenBao
Bump ONNX in CI to resolve failures on "[Migration][DO NOT MERGE] Sep…
49c4104 
…arate old ir into `_legacy_ir` folder"

All tests except for linter are expected to pass.

[ghstack-poisoned]
@BowenBao BowenBao mentioned this pull request Apr 2, 2024
Closed
@BowenBao
Update on "[Migration][DO NOT MERGE] Separate old ir into `_legacy_ir…
d5ab484 
…` folder"

All tests except for linter are expected to pass.

[ghstack-poisoned]
justinchuby
justinchuby approved these changes Apr 2, 2024
View reviewed changes
@BowenBao BowenBao mentioned this pull request Apr 3, 2024
Merged
@BowenBao
rebase with regression fix on "[Migration][DO NOT MERGE] Separate old…
d0b0c34 
… ir into `_legacy_ir` folder"

All tests except for linter are expected to pass.

[ghstack-poisoned]
This was referenced Apr 3, 2024
Closed
Closed
BowenBao added a commit that referenced this pull request Apr 4, 2024
@BowenBao
Migrate onnxrewriter
34a345f 
Squashed of the following steps:
- #1328
- #1329
- #1330
- #1331
- #1332
- #1333
- #1343
- #1345

Co-authored-by: Shubham Bhokare <[email protected]>
Co-authored-by: Justin Chu <[email protected]>
Co-authored-by: Xavier Dupré <[email protected]>
Co-authored-by: "G. Ramalingam" <[email protected]>
Co-authored-by: kunal-vaishnavi <[email protected]>
Co-authored-by: Ti-Tai Wang <[email protected]>

[ghstack-poisoned]
@BowenBao BowenBao mentioned this pull request Apr 4, 2024
Merged
BowenBao added a commit that referenced this pull request Apr 5, 2024
@BowenBao
Attempt to fix linter on "Migrate onnxrewriter"
667ab08 
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 added a commit that referenced this pull request Apr 5, 2024
@BowenBao
Update on "Migrate onnxrewriter"
596f3af 
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 added a commit that referenced this pull request Apr 5, 2024
@BowenBao @justinchuby
@xadupre @gramalingam @titaiwangms
Migrate onnxrewriter (#1346)
2c74be7 
Stack from [ghstack](https://github.com/ezyang/ghstack) (oldest at
bottom):
* #1334
* #1340
* __->__ #1346

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

Co-authored-by: Shubham Bhokare
<[email protected]>
Co-authored-by: Justin Chu <[email protected]>
Co-authored-by: Xavier Dupré <[email protected]>
Co-authored-by: "G. Ramalingam" <[email protected]>
Co-authored-by: kunal-vaishnavi
<[email protected]>
Co-authored-by: Ti-Tai Wang <[email protected]>
@BowenBao
Copy link
Contributor Author

BowenBao commented Apr 5, 2024

Close as squashed and merged into main.

@BowenBao BowenBao closed this Apr 5, 2024
@justinchuby justinchuby deleted the gh/BowenBao/24/head branch January 27, 2025 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinchuby justinchuby justinchuby approved these changes

Assignees
No one assigned
Labels
None yet
Projects
ONNX Script Review Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BowenBao @justinchuby