Skip to content

[Migration][DO NOT MERGE] Restructure rewriter core under onnxscript #1329

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
BowenBao wants to merge 4 commits into from
Closed

[Migration][DO NOT MERGE] Restructure rewriter core under onnxscript #1329

BowenBao wants to merge 4 commits into from

Conversation

@BowenBao
Copy link
Contributor

@BowenBao BowenBao commented Apr 1, 2024 •
edited
Loading

Stack from ghstack (oldest at bottom):

  • Moving files to new directories without changing.
  • Merging ir/init.py with existing ir/init.py. The two IRs are expected to be merged in the future.

@BowenBao
[Migration] Restructure rewriter core under onnxscript
50564ad 
- Moving files to new directories without changing.
- Merging ir/__init__.py with existing ir/__init__.py. The two IRs are expected to be merged in the future.

[ghstack-poisoned]
This was referenced Apr 1, 2024
Closed
Closed
Closed
@BowenBao BowenBao changed the title [Migration] Restructure rewriter core under onnxscript [Migration][DO NOT MERGE] Restructure rewriter core under onnxscript Apr 1, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 1, 2024
View reviewed changes
onnxscript/ir/__init__.py
)


Value = StaticValueInfo

Check warning

Code scanning / lintrunner

RUFF/F811

Redefinition of unused `Value` from line 88. See https://docs.astral.sh/ruff/rules/redefined-while-unused
onnxscript/ir/__init__.py
Value = StaticValueInfo


class Model:

Check warning

Code scanning / lintrunner

RUFF/F811

Redefinition of unused `Model` from line 79. See https://docs.astral.sh/ruff/rules/redefined-while-unused
onnxscript/ir/__init__.py
)


class Graph:

Check warning

Code scanning / lintrunner

RUFF/F811

Redefinition of unused `Graph` from line 77. See https://docs.astral.sh/ruff/rules/redefined-while-unused
onnxscript/ir/__init__.py
)


class Function:

Check warning

Code scanning / lintrunner

RUFF/F811

Redefinition of unused `Function` from line 76. See https://docs.astral.sh/ruff/rules/redefined-while-unused
onnxscript/ir/__init__.py
)


class RefAttr:

Check warning

Code scanning / lintrunner

RUFF/F811

Redefinition of unused `RefAttr` from line 82. See https://docs.astral.sh/ruff/rules/redefined-while-unused
onnxscript/ir/__init__.py
ValueProtocol,
)

import dataclasses

Check notice

Code scanning / lintrunner

PYLINT/C0411

standard import "import dataclasses" should be placed before "from onnxscript.ir import serde" (wrong-import-order) See [wrong-import-order](https://pylint.pycqa.org/en/latest/user_guide/messages/convention/wrong-import-order.html). To disable, use ` # pylint: disable=wrong-import-order`
onnxscript/ir/__init__.py
)

import dataclasses
from collections import deque

Check notice

Code scanning / lintrunner

PYLINT/C0411

standard import "from collections import deque" should be placed before "from onnxscript.ir import serde" (wrong-import-order) See [wrong-import-order](https://pylint.pycqa.org/en/latest/user_guide/messages/convention/wrong-import-order.html). To disable, use ` # pylint: disable=wrong-import-order`
onnxscript/ir/__init__.py

import dataclasses
from collections import deque
from typing import List, Tuple, Union

Check notice

Code scanning / lintrunner

PYLINT/C0411

standard import "from typing import List, Tuple, Union" should be placed before "from onnxscript.ir import serde" (wrong-import-order) See [wrong-import-order](https://pylint.pycqa.org/en/latest/user_guide/messages/convention/wrong-import-order.html). To disable, use ` # pylint: disable=wrong-import-order`
onnxscript/ir/__init__.py
from collections import deque
from typing import List, Tuple, Union

import numpy as np

Check notice

Code scanning / lintrunner

PYLINT/C0411

third party import "import numpy as np" should be placed before "from onnxscript.ir import serde" (wrong-import-order) See [wrong-import-order](https://pylint.pycqa.org/en/latest/user_guide/messages/convention/wrong-import-order.html). To disable, use ` # pylint: disable=wrong-import-order`
onnxscript/ir/__init__.py
from typing import List, Tuple, Union

import numpy as np
import onnx

Check notice

Code scanning / lintrunner

PYLINT/C0411

third party import "import onnx" should be placed before "from onnxscript.ir import serde" (wrong-import-order) See [wrong-import-order](https://pylint.pycqa.org/en/latest/user_guide/messages/convention/wrong-import-order.html). To disable, use ` # pylint: disable=wrong-import-order`
@codecov
Copy link

codecov bot commented Apr 1, 2024 •
edited
Loading

Codecov Report

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

Project coverage is 61.28%. Comparing base (df60e25) to head (9809ec0).

Files Patch % Lines
onnxscript/ir/__init__.py 45.51% 84 Missing and 1 partial ⚠️
Additional details and impacted files 
@@                   Coverage Diff                   @@
##           gh/BowenBao/21/base    #1329      +/-   ##
=======================================================
- Coverage                61.82%   61.28%   -0.54%     
=======================================================
  Files                      183      182       -1     
  Lines                    22001    22001              
  Branches                  3646     3646              
=======================================================
- Hits                     13602    13484     -118     
- Misses                    7954     8118     +164     
+ Partials                   445      399      -46

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

@justinchuby
Copy link
Collaborator

justinchuby commented Apr 1, 2024

I would rename the original ir to be legacy_ etc. and store in the separate folder.

@github-actions
Copy link

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

Test Results

     24 files  ±    0      24 suites  ±0   2h 36m 51s ⏱️ + 2m 49s
 13 009 tests +1 250   9 386 ✅ +960    3 585 💤 +271   20 ❌ + 19   18 🔥 ±0 
412 039 runs  +  273  96 821 ✅  - 357  314 209 💤 ±  0  631 ❌ +630  378 🔥 ±0 

For more details on these failures and errors, see this check.

Results for commit 9809ec0. ± Comparison against base commit df60e25.

♻️ This comment has been updated with latest results.

This was referenced Apr 1, 2024
Closed
Closed
Closed
@BowenBao
Bump ONNX in CI to resolve failures on "[Migration][DO NOT MERGE] Res…
a7a89bc 
…tructure rewriter core under onnxscript"

- Moving files to new directories without changing.
- Merging ir/__init__.py with existing ir/__init__.py. The two IRs are expected to be merged in the future.

[ghstack-poisoned]
@BowenBao BowenBao mentioned this pull request Apr 2, 2024
Closed
@BowenBao
Update on "[Migration][DO NOT MERGE] Restructure rewriter core under …
7520717 
…onnxscript"

- Moving files to new directories without changing.
- Merging ir/__init__.py with existing ir/__init__.py. The two IRs are expected to be merged in the future.

[ghstack-poisoned]
@BowenBao BowenBao mentioned this pull request Apr 3, 2024
Merged
@BowenBao
rebase with regression fix on "[Migration][DO NOT MERGE] Restructure …
9809ec0 
…rewriter core under onnxscript"

- Moving files to new directories without changing.
- Merging ir/__init__.py with existing ir/__init__.py. The two IRs are expected to be merged in the future.

[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/21/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.

3 participants

@BowenBao @justinchuby