Skip to content

Commit ee6e22f

Browse files
committed
Update on "[ONNX] Introduce FX-ONNX dispatcher"
Needs microsoft/onnxscript#721 The current FX exporter is using manually maintained dictionary to map ATen op to its OnnxFunction. However, the issue arises when ATen op has overloads or OnnxFunction has overloads, which is not resolvable by the one to one mapping . For example, `aten::arange` has onverloads: `aten::arange.start` and `aten::arange.start_step`, or for `aten::argmax`, torchlib provides two function: aten_argmax, and aten_argmax_dim. This PR utilizes newly introduced [ONNX OpSchema](microsoft/onnxscript#626) to match the input arguments of an ATen operator to find the correct overload. ### OnnxRegistry Heavily reference on [TorchScript Registry](#84382). The only difference is that in FX registry, an ATen operator with specific opset version is mapped to a list of overloaded functions. * No longer use global registry. The registry is initialized in `ResolvedExportOptions` with torchlib, and will be exposed to users in the future. * Multiple opset version layer is kept through `_SymbolicFunctionGroup` , but torchlib now only supports 18. * Basic API of custom operator support: `register`, `unregister`, and `is_register_op` are kept for future development. To further complete them, the follow-up PRs should address: - How to allow users to remove/override specific overload? Using OpSchema to differentiate? - User registers a new overload with the same OpSchema as one of registered overload. ### OnnxDispatcher Dispatch ATen operators to the matched overload by comparing OpSchema with input arguments. * `OpSchemaWrapper` wrap the onnx schema, and record matching score. * `dispatch` uses `OpSchemaWrapper` to compare data types to find the best matched overload. If the match isn't perfect, record warning in diagnostics. * `dispatch_opset_version` is referenced from #84382 and kept, but torchlib doesn't support opset version != 18. * Because right now (1) OnnxFunction arguments are manually typed, and (2) ORT could unfollow ONNX type spec, we relax the schema match with `matching score system`. * To include more supports: the follow-up PRs should address: - How to add op.Cast with autocast? In torchlib or converter? - The need of type promotion can be captured by dispatcher, but needs OpSchema shows the T1/T2 information. ### OpSchemaWrapper - Matching Score Mechanism #### The matching score system: This is a temporary solution to how we target the correct ONNX overloads given that we only have manually annotated arguments (potentially inaccurate schema) and limited supports on AttributeProto. 1. Perfect match exam: If all arguments/kwargs are all matched, return the function without any warnings. 2. Best match exam: The system add the each correct matching input counts orderly, and subtract the symmetrical difference between their attributes to calculate the matching score. And select the one with the highest score in the end. If the selection is not a perfect match, a warning message is sent to SARIF. #### Example of overloads 1. Different types: Caused by the difference between the ONNX spec and PyTorch. The matching system finds the correct one. ```python torch_op("aten::mul") def aten_mul(self: TReal, other: TReal) -> TReal: ... torch_op("aten::mul") def aten_mul_bool(self: BOOL, other: BOOL) -> BOOL: ... ``` 2. Optional dim: caused by unsupported op.OptionalHasElement (will support on opset version == 20). dim could be "None" ```python torch_op("aten::argmax", trace_only=True) def aten_argmax( self: TrealOrUInt8, dim: Optional[int] = None, keepdim: bool = False ) -> TrealOrUInt8: ... torch_op("aten::argmax", private=True) def _aten_argmax_dim(self: TrealOrUInt8, dim: int, keepdim: bool = False) -> TrealOrUInt8: ... ``` This case is impossible to differentiate, as they both might have dim in kwargs, so in this case, please make sure you turn the one with `dim: int` to private function. 3. Optional dtype: dtype could be "unprovided". The difference from 2 is that dtype would not be None. ```python torch_op("aten::new_full") def aten_new_full(self: TTensor, size: INT64, fill_value: TTensor) -> TTensor: ... torch_op("aten::new_full") def aten_new_full_dtype(self: TTensor, size: INT64, fill_value: TTensor, dtype: int) -> TTensor: ... ``` Depends on dtype is provided or not, matching system will dispatch the ATen op to the correct one. 4. `None` and `[]` and `NoneType` are considered failing the match. 5. Two functions have the same score is recorded into SARIFs. ### TODOs 1. Type promotion can be captured by dispatcher only if OpSchema can provide it. However, the implementation of "graph-level" pass vs "in-op"" promotion can be further discussed in microsoft/onnxscript#563. 5. torchlib should provide the "opset version" to OnnxRegistry. 7. How to expose OnnxRegistry with custom add/remove ops APIs nneds to be further discussed. Co-authored-by: Justin Chu <justinchubymicrosoft.com> [ghstack-poisoned]
2 parents 153ba39 + 282518a commit ee6e22f

File tree

285 files changed

+13427
-5906
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

285 files changed

+13427
-5906
lines changed

.github/ci_commit_pins/torchbench.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
44ccad1d0647d56fe4e56c0b1933102be5dcc874
1+
f4acd1a7fcce986155c5e20beffa92b24ae0a3fa

.github/ci_commit_pins/vision.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
99ec261c72c097160e94653cce6f90f2d1209222
1+
d2f7486ccaef461913cdb51990ff353addf6f064

.github/scripts/label_utils.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
BOT_AUTHORS = ["github-actions", "pytorchmergebot", "pytorch-bot"]
1717

18-
LABEL_ERR_MSG_TITLE = "This PR needs a label"
18+
LABEL_ERR_MSG_TITLE = "This PR needs a `release notes:` label"
1919
LABEL_ERR_MSG = f"""# {LABEL_ERR_MSG_TITLE}
2020
If your changes are user facing and intended to be a part of release notes, please use a label starting with `release notes:`.
2121
@@ -111,7 +111,9 @@ def has_required_labels(pr: "GitHubPR") -> bool:
111111

112112

113113
def is_label_err_comment(comment: GitHubComment) -> bool:
114+
# comment.body_text returns text without markdown
115+
no_format_title = LABEL_ERR_MSG_TITLE.replace("`", "")
114116
return (
115-
comment.body_text.lstrip(" #").startswith(LABEL_ERR_MSG_TITLE)
117+
comment.body_text.lstrip(" #").startswith(no_format_title)
116118
and comment.author_login in BOT_AUTHORS
117119
)

.github/scripts/test_check_labels.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ def mock_get_comments() -> List[GitHubComment]:
4444
),
4545
# Case 2 - a label err comment
4646
GitHubComment(
47-
body_text=" #" + LABEL_ERR_MSG_TITLE,
47+
body_text=" #" + LABEL_ERR_MSG_TITLE.replace("`", ""),
4848
created_at="",
4949
author_login=BOT_AUTHORS[1],
5050
author_association="",

.github/scripts/trymerge.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,9 +1262,9 @@ def find_matching_merge_rule(
12621262
reject_reason_score = num_matching_files
12631263
reject_reason = "\n".join(
12641264
(
1265-
f"Not all files match rule `{rule_name}`."
1266-
f"{num_matching_files} files matched, but there are still non-matching files:"
1267-
f"{','.join(non_matching_files[:5])}{', ...' if len(non_matching_files) > 5 else ''}"
1265+
f"Not all files match rule `{rule_name}`.",
1266+
f"{num_matching_files} files matched, but there are still non-matching files:",
1267+
f"{','.join(non_matching_files[:5])}{', ...' if len(non_matching_files) > 5 else ''}",
12681268
)
12691269
)
12701270
continue

.github/scripts/update_commit_hashes.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66

77
import requests
88

9-
MERGEBOT_TOKEN = os.environ["MERGEBOT_TOKEN"]
9+
UPDATEBOT_TOKEN = os.environ["UPDATEBOT_TOKEN"]
1010
PYTORCHBOT_TOKEN = os.environ["PYTORCHBOT_TOKEN"]
1111
OWNER, REPO = "pytorch", "pytorch"
1212

1313

1414
def git_api(
15-
url: str, params: Dict[str, str], type: str = "get", token: str = MERGEBOT_TOKEN
15+
url: str, params: Dict[str, str], type: str = "get", token: str = UPDATEBOT_TOKEN
1616
) -> Any:
1717
headers = {
1818
"Accept": "application/vnd.github.v3+json",

.github/workflows/_update-commit-hash.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ on:
2222
required: false
2323
default: .github/ci_commit_pins
2424
secrets:
25-
MERGEBOT_TOKEN:
25+
UPDATEBOT_TOKEN:
2626
required: true
2727
description: Permissions for opening PR
2828
PYTORCHBOT_TOKEN:
@@ -41,7 +41,7 @@ jobs:
4141
with:
4242
fetch-depth: 1
4343
submodules: false
44-
token: ${{ secrets.MERGEBOT_TOKEN }}
44+
token: ${{ secrets.UPDATEBOT_TOKEN }}
4545

4646
- name: Checkout
4747
shell: bash
@@ -54,11 +54,11 @@ jobs:
5454
REPO_NAME: ${{ inputs.repo-name }}
5555
BRANCH: ${{ inputs.branch }}
5656
PIN_FOLDER: ${{ inputs.pin-folder }}
57-
MERGEBOT_TOKEN: ${{ secrets.MERGEBOT_TOKEN }}
57+
UPDATEBOT_TOKEN: ${{ secrets.UPDATEBOT_TOKEN }}
5858
PYTORCHBOT_TOKEN: ${{ secrets.PYTORCHBOT_TOKEN }}
5959
run: |
6060
# put this here instead of the script to prevent accidentally changing the config when running the script locally
61-
git config --global user.name "PyTorch MergeBot"
62-
git config --global user.email "pytorchmergebot@users.noreply.github.com"
61+
git config --global user.name "PyTorch UpdateBot"
62+
git config --global user.email "pytorchupdatebot@users.noreply.github.com"
6363
6464
python .github/scripts/update_commit_hashes.py --repo-name "${REPO_NAME}" --branch "${BRANCH}" --pin-folder "${PIN_FOLDER}"

.github/workflows/check-labels.yml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,22 @@
11
name: Check Labels
22

33
on:
4+
# We need pull_request_target to be able to post comments on PRs from forks.
5+
# Only allow pull_request_target when merging to main, not some historical branch.
6+
#
7+
# Make sure to don't introduce explicit checking out and installing/running
8+
# untrusted user code into this workflow!
9+
pull_request_target:
10+
types: [opened, synchronize, reopened, labeled, unlabeled]
11+
branches: [main]
12+
paths-ignore: [.github]
13+
14+
# To allow testing PRs that change workflows.
15+
# May be triggered together with pull_request_target, it's OK.
416
pull_request:
517
types: [opened, synchronize, reopened, labeled, unlabeled]
18+
paths: [.github]
19+
620
workflow_dispatch:
721

822
concurrency:

.github/workflows/inductor.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ jobs:
7373
docker-image-name: pytorch-linux-focal-py3.8-gcc7
7474
test-matrix: |
7575
{ include: [
76-
{ config: "inductor_huggingface_cpu_accuracy", shard: 1, num_shards: 1, runner: "linux.4xlarge" },
76+
{ config: "inductor_huggingface_cpu_accuracy", shard: 1, num_shards: 1, runner: "linux.12xlarge" },
7777
{ config: "inductor_timm_cpu_accuracy", shard: 1, num_shards: 2, runner: "linux.4xlarge" },
7878
{ config: "inductor_timm_cpu_accuracy", shard: 2, num_shards: 2, runner: "linux.4xlarge" },
7979
{ config: "inductor_torchbench_cpu_accuracy", shard: 1, num_shards: 1, runner: "linux.4xlarge" },

.github/workflows/lint-bc.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ on:
88
- reopened
99
- labeled
1010
- unlabeled
11+
branches-ignore:
12+
- nightly
1113
workflow_dispatch:
1214

1315
jobs:

0 commit comments

Comments
 (0)