Document supported ops #1475

AlexandreEichenberger · 2022-05-31T23:50:56Z

Add format to testing file to register our current support. Example of format:

################################################################################
# SEMANTIC for LABELING (one line per directive)
#
# ==ARCH== <arch>
#   where <arch> is cpu/NNPA/... this option is valid until reset by another ARCH dir
#
# ==OP== <op> <text>
#   where <op> is the ONNX op name
#   where <text> qualifies the opset currently being supported. When "current" is 
#   provided, the postprocessing will automatically changed highest opset currently
#   supported. When no <text> is provided, the operation is assumed to be fully 
#   unsupported.
#
# ==LIM== <text> 
#   where <text> qualifies the current restrictions to the implementation.
#
# ==TODO== <text>
#   where <text> add "private" info about what needs to be fixed. 
#

Where this info will be scanned, parsed, and generate a md file listing our current support

Signed-off-by: Alexandre Eichenberger <[email protected]>

AlexandreEichenberger · 2022-06-08T20:50:11Z

@chentong319 now using the version info from gen_onnx_mlir

Signed-off-by: Alexandre Eichenberger <[email protected]>

AlexandreEichenberger · 2022-06-09T16:04:41Z

@cjvolzka Please check if the output generated is ok by you. You can visualize this md file. docs/SupportedONNXOps-cpu.md

chentong319 · 2022-06-09T19:07:28Z

At the beginning of this project, only a small portion of backend tests are supported. Therefore, we created a dictionary for enabled tests. Now it is the time to assume the default status of a test is enabled for static, dynamic and constant with every dimension ({-1:{-1}}). Then we can have a dictionary for test cases that can not enabled with the default value. An entry in the dictionary (for one test case) is also a dictionary with following possible entries:

Key "Disabled": string, why this test is disabled
Key " DYNAMIC_SHAPE" : the dimension string
Key "CONSTANT_SHAPE" : then dimension string

Benefits:

Compare the all_test_names (line 68 test.py) and this dictionary, we can get which op is supported and which is not
Easier to enable new test case without going into third_part/onnx to figure out the test names
Reduce the redundancy in dimension info: most of them are the default value
The reason becomes part of the python code, not just comments.

Should we open a new PR after this PR is closed or merged?

AlexandreEichenberger · 2022-06-09T23:34:19Z

Should we open a new PR after this PR is closed or merged?

The development team has expressed interest in having the list of supported ops on the GitHub pages, so I think we should merge it in, then you can change the testing logic as you see fit and I will find a way to recreate the info, using either the same or slightly different annotations.

chentong319 · 2022-06-10T00:55:08Z

Should we open a new PR after this PR is closed or merged?

The development team has expressed interest in having the list of supported ops on the GitHub pages, so I think we should merge it in, then you can change the testing logic as you see fit and I will find a way to recreate the info, using either the same or slightly different annotations.

Sure. I will review this PR.

chentong319

Should we in general put the update of the component of third_part in an independent PR?

chentong319 · 2022-06-10T01:04:59Z

utils/documentOps.py

+#
+################################################################################
+#
+# This file convert .mlir from stdin into a FileCheck file format. It also


Update the header comment, which seems to be copied from mlir2FileCheck.py

good catch, sorry.

chentong319 · 2022-06-10T01:13:44Z

utils/documentOps.py

+        if arch != target_arch:
+            continue
+        # Scan op (op followed by any text).
+        p = re.search(r'==OP==\s+(\w+)', l)


The op-set for an Op in ONNX dialect is determined by gen_mlir_onnx.py. If that op-set is not the version specified in the version of onnx we are using, the backend test always goes with the onnx. It is a little confusion to say which op-set we are supporting. Anyway, we should not need to specify op-set in the backend test. Let's fix it in future PR.

As a matter of fact, I removed the text after the ==OP== as you rightly suggested to use the gen_onnx_mlir.py info on the opsets.

chentong319 · 2022-06-10T01:22:10Z

utils/gen_onnx_mlir.py

-                            .format(schema.name)+
-                            " has a newer version {} over old version {}"
-                            .format(schema.since_version, version_dict[schema.name][0]))
+                    if not list_only:


Did I miss some code? I did not see the extra work when list_only is true. I just wonder that you print out all the versions of an Op or just the top version?

I should have given more input. The current gen_onnx_mlir.py --check-operation-version gives additional output, which is disabled when list_only is on. The reason I needed no additional output is that in the documentOps, I run a subprocess that run gen_onnx_mlir.py --check-operation-version which print a dictionary, which I then evaluate in the current python script to gather the proper info. I could not simply include that script as both scripts needs to parse distinct input arguments from the command line.

After discussions with @chentong319 , we are now using directly the gen_onnx_mlir version_dict directly. He also requested that we only report the largest opset (and not the list of opsets). All these changes have now been implemented.

cjvolzka · 2022-06-10T02:13:41Z

@cjvolzka Please check if the output generated is ok by you. You can visualize this md file. docs/SupportedONNXOps-cpu.md

Overall looks good. I think it'll be useful info to have but also for debugging. For example if a model doesn't work, it should be easy to reference what Ops are supported and then tell if you have an unsupported op.

I do have a couple of small comments/questions related to the column headers:

Opset: Does this need a little more clarity?
- For example on "Add" it only shows Opset 14. Does that mean we don't support it for other Opsets or we just only tested it on 14? It's a little ambiguous to me because some Ops (like "Clip") we explicitly list specific Opsets but most others we only list one. However then on "Loop" we only show 16 but then explicitly call out Opsets that aren't supported.
Todo: Should be renamed to something like Notes

chentong319 · 2022-06-10T14:12:47Z

@cjvolzka Please check if the output generated is ok by you. You can visualize this md file. docs/SupportedONNXOps-cpu.md

Overall looks good. I think it'll be useful info to have but also for debugging. For example if a model doesn't work, it should be easy to reference what Ops are supported and then tell if you have an unsupported op.

I do have a couple of small comments/questions related to the column headers:

Opset: Does this need a little more clarity?

For example on "Add" it only shows Opset 14. Does that mean we don't support it for other Opsets or we just only tested it on 14? It's a little ambiguous to me because some Ops (like "Clip") we explicitly list specific Opsets but most others we only list one. However then on "Loop" we only show 16 but then explicitly call out Opsets that aren't supported.

Todo: Should be renamed to something like Notes

Good suggestion. We will let our model importer read this table and generate error message when there is an unsupported Op in the model.

AlexandreEichenberger · 2022-06-10T15:36:44Z

@chentong319

Something I don't understand with the gen_onnx_mlir script. If you look at loop op, its listed as 13 but when you run the script it list it at 16.

python ../utils/gen_onnx_mlir.py --check-operation-version | grep Loop
Check-operation-version: Operation Loop has a newer version 16 over old version 13
 'Loop': [16],

Can you show me how to preserve the '13' for this PR?

UPDATE: now implemented, all good.

AlexandreEichenberger · 2022-06-10T15:39:33Z

@cjvolzka Thanks for looking into the output.

Opset: Does this need a little more clarity?

Its the same as in the standard: it list the LAST opset that "improved" the given operation. So Add is in 16, but it was last modified in 14, so it list 14.

I will give it an explanation.

For loop, we support the "syntax" of 13, but only when the syntax satisfies some constraints, namely that of 9.

Signed-off-by: Alexandre Eichenberger <[email protected]>

chentong319 · 2022-06-10T16:35:23Z

There is a version number for onnx. For example, we are using onnx.1.11.0. In one onnx version, ops may have different opset number, depending on whether the Op is modified or not. Sometimes the highest opset number in a version is used to denote that onnx version.
In onnx-mlir compiler, we should have a table for the opset number of each Op we support, and the current opset number of each Op in the version of onnx we are using. Currently we have the opset number only in gen_onnx_mlir.py, not in the compiler. Warning and conversion in our Builder should be based on the two tables, not just one opset number.
Not sure whether we want to fix this issue in this PR or not. @AlexandreEichenberger , it's your call.

chentong319

LGTM!

Signed-off-by: Alexandre Eichenberger <[email protected]>

AlexandreEichenberger · 2022-06-10T19:39:33Z

In onnx-mlir compiler, we should have a table for the opset number of each Op we support, and the current opset number of each Op in the version of onnx we are using. Currently we have the opset number only in gen_onnx_mlir.py, not in the compiler. Warning and conversion in our Builder should be based on the two tables, not just one opset number.

For the sake of expediency, this PR implement the right thing for the doc. Will tabulate larger changes for supporting opset to a later PR.

chentong319

LGTM!

chentong319

Another try to approve

chentong319 · 2022-06-13T14:24:01Z

Whenever I submitted an approval, the approval was turned into a comment. Something wrong with my git account. I could not merge my own approved PR, either.

chentong319

Again!

jenkins-droid · 2022-06-13T14:44:31Z

Jenkins Linux s390x Build #6355 [push] Document supported ops (... started at 10:44

jenkins-droid · 2022-06-13T14:44:33Z

Jenkins Linux ppc64le Build #5477 [push] Document supported ops (... started at 10:46

jenkins-droid · 2022-06-13T14:44:35Z

Jenkins Linux amd64 Build #6339 [push] Document supported ops (... started at 09:44

jenkins-droid · 2022-06-13T15:52:38Z

Jenkins Linux amd64 Build #6339 [push] Document supported ops (... passed after 1 hr 8 min

jenkins-droid · 2022-06-13T16:03:53Z

Jenkins Linux s390x Build #6355 [push] Document supported ops (... passed after 1 hr 19 min

jenkins-droid · 2022-06-13T16:25:35Z

Jenkins Linux ppc64le Build #5477 [push] Document supported ops (... passed after 1 hr 41 min

AlexandreEichenberger added 3 commits May 31, 2022 13:51

first proto

2371974

Signed-off-by: Alexandre Eichenberger <[email protected]>

Merge branch 'main' into document-supported-ops

4c9b21d

first draft

181cca2

Signed-off-by: Alexandre Eichenberger <[email protected]>

AlexandreEichenberger marked this pull request as draft May 31, 2022 23:51

AlexandreEichenberger linked an issue Jun 2, 2022 that may be closed by this pull request

Supported opset doc? #1457

Closed

AlexandreEichenberger added 11 commits June 2, 2022 23:58

Merge branch 'main' into document-supported-ops

445711a

update

b29f5f2

Signed-off-by: Alexandre Eichenberger <[email protected]>

update

2a1a7f8

Signed-off-by: Alexandre Eichenberger <[email protected]>

Merge branch 'main' into document-supported-ops

dbea8e8

update

9fd86db

Signed-off-by: Alexandre Eichenberger <[email protected]>

supported ops

1154e73

Signed-off-by: Alexandre Eichenberger <[email protected]>

supported ops

8170ee1

Signed-off-by: Alexandre Eichenberger <[email protected]>

Merge branch 'main' into document-supported-ops

cbc3dd1

update

9f5c45d

Signed-off-by: Alexandre Eichenberger <[email protected]>

update

540ad09

Signed-off-by: Alexandre Eichenberger <[email protected]>

remove test that failed JNI test

2074853

Signed-off-by: Alexandre Eichenberger <[email protected]>

AlexandreEichenberger marked this pull request as ready for review June 8, 2022 13:09

AlexandreEichenberger added 5 commits June 8, 2022 13:09

update

b15b5fa

Signed-off-by: Alexandre Eichenberger <[email protected]>

Merge branch 'main' into document-supported-ops

f2f178d

update using info from gen_onnx_mlir

ad02510

Signed-off-by: Alexandre Eichenberger <[email protected]>

new scheme

e94a12a

Signed-off-by: Alexandre Eichenberger <[email protected]>

Merge branch 'main' into document-supported-ops

e19a0d2

AlexandreEichenberger added 2 commits June 9, 2022 15:43

Merge branch 'main' into document-supported-ops

ec34448

cleanup

dbe2ef4

Signed-off-by: Alexandre Eichenberger <[email protected]>

AlexandreEichenberger requested review from chentong319, etiotto and cjvolzka June 9, 2022 15:46

chentong319 reviewed Jun 10, 2022

View reviewed changes

responce to comments in PR

ba2cf10

Signed-off-by: Alexandre Eichenberger <[email protected]>

chentong319 approved these changes Jun 10, 2022

View reviewed changes

change to use opset number that is actually supported

76f4229

Signed-off-by: Alexandre Eichenberger <[email protected]>

chentong319 approved these changes Jun 11, 2022

View reviewed changes

chentong319 self-requested a review June 13, 2022 14:07

chentong319 approved these changes Jun 13, 2022

View reviewed changes

chentong319 merged commit c192123 into onnx:main Jun 13, 2022

AlexandreEichenberger mentioned this pull request Sep 15, 2022

[RFC] Adding an ONNX to Torch conversion #1639

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document supported ops #1475

Document supported ops #1475

AlexandreEichenberger commented May 31, 2022 •

edited

Loading

AlexandreEichenberger commented Jun 8, 2022

AlexandreEichenberger commented Jun 9, 2022

chentong319 commented Jun 9, 2022 •

edited

Loading

AlexandreEichenberger commented Jun 9, 2022

chentong319 commented Jun 10, 2022

chentong319 left a comment

chentong319 Jun 10, 2022

AlexandreEichenberger Jun 10, 2022

chentong319 Jun 10, 2022

AlexandreEichenberger Jun 10, 2022

chentong319 Jun 10, 2022

AlexandreEichenberger Jun 10, 2022

AlexandreEichenberger Jun 10, 2022

cjvolzka commented Jun 10, 2022

chentong319 commented Jun 10, 2022

AlexandreEichenberger commented Jun 10, 2022 •

edited

Loading

AlexandreEichenberger commented Jun 10, 2022

chentong319 commented Jun 10, 2022

chentong319 left a comment

AlexandreEichenberger commented Jun 10, 2022

chentong319 left a comment

chentong319 left a comment

chentong319 commented Jun 13, 2022

chentong319 left a comment

jenkins-droid commented Jun 13, 2022

jenkins-droid commented Jun 13, 2022

jenkins-droid commented Jun 13, 2022

jenkins-droid commented Jun 13, 2022

jenkins-droid commented Jun 13, 2022

jenkins-droid commented Jun 13, 2022

Document supported ops #1475

Document supported ops #1475

Conversation

AlexandreEichenberger commented May 31, 2022 • edited Loading

AlexandreEichenberger commented Jun 8, 2022

AlexandreEichenberger commented Jun 9, 2022

chentong319 commented Jun 9, 2022 • edited Loading

AlexandreEichenberger commented Jun 9, 2022

chentong319 commented Jun 10, 2022

chentong319 left a comment

Choose a reason for hiding this comment

chentong319 Jun 10, 2022

Choose a reason for hiding this comment

AlexandreEichenberger Jun 10, 2022

Choose a reason for hiding this comment

chentong319 Jun 10, 2022

Choose a reason for hiding this comment

AlexandreEichenberger Jun 10, 2022

Choose a reason for hiding this comment

chentong319 Jun 10, 2022

Choose a reason for hiding this comment

AlexandreEichenberger Jun 10, 2022

Choose a reason for hiding this comment

AlexandreEichenberger Jun 10, 2022

Choose a reason for hiding this comment

cjvolzka commented Jun 10, 2022

chentong319 commented Jun 10, 2022

AlexandreEichenberger commented Jun 10, 2022 • edited Loading

AlexandreEichenberger commented Jun 10, 2022

chentong319 commented Jun 10, 2022

chentong319 left a comment

Choose a reason for hiding this comment

AlexandreEichenberger commented Jun 10, 2022

chentong319 left a comment

Choose a reason for hiding this comment

chentong319 left a comment

Choose a reason for hiding this comment

chentong319 commented Jun 13, 2022

chentong319 left a comment

Choose a reason for hiding this comment

jenkins-droid commented Jun 13, 2022

jenkins-droid commented Jun 13, 2022

jenkins-droid commented Jun 13, 2022

jenkins-droid commented Jun 13, 2022

jenkins-droid commented Jun 13, 2022

jenkins-droid commented Jun 13, 2022

AlexandreEichenberger commented May 31, 2022 •

edited

Loading

chentong319 commented Jun 9, 2022 •

edited

Loading

AlexandreEichenberger commented Jun 10, 2022 •

edited

Loading