Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

fix(speedup): re-write aten schema parser to support pytorch versions < 1.9.0 #5138

Merged
merged 5 commits into from
Oct 18, 2022

Conversation

Louis-J
Copy link
Contributor

@Louis-J Louis-J commented Sep 23, 2022

Description

reason:
the official aten schema parser of python lost the 'kwarg_only' info in python versions <1.9.0.
and schema is not simple enough to use some regex to parser.

solution:
so I translated the schema parser code from pytorch 1.12 c++ code to python and adapted some deleted syntaxes in pytorch 1.8.
tested in pytorch versions 1.8, 1.9, 1.10, 1.11 and 1.12.

note:
torch._C.Node supports schema function since version 1.8. so speedup cannot use on pytorch 1.7 now.
#5131 can be solved.

Test Options

  • fast test
  • full test - HPO
  • full test - NAS
  • full test - compression

Checklist

  • test case
  • doc

How to test

@Louis-J Louis-J changed the title WIP: 837a2920 add an example fix(speedup): re-write aten schema parser to support pytorch versions < 1.9.0 Sep 23, 2022
@liuzhe-lz
Copy link
Contributor

liuzhe-lz commented Oct 11, 2022

I suggest to keep both version and use a flag to switch.
In future it's very likely that nobody can maintain this code. At that time dropping certain version's support will be a more practical choice.

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@Louis-J
Copy link
Contributor Author

Louis-J commented Oct 13, 2022

/azp run full test - compression

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@Louis-J Louis-J requested a review from J-shang October 14, 2022 09:19
@@ -486,7 +748,10 @@ def generate_aten_to_python(func: Callable, node: NodePyGroup, speedup: ModelSpe
c_node = node.key_node

schema = c_node.schema()
positional_num, keyword_list, special_treat = parse_aten_schema(schema)
if torch.__version__ < '1.9.0':
Copy link
Contributor

@J-shang J-shang Oct 17, 2022

Choose a reason for hiding this comment

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

image

I'm worried with this method of judging version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the type of torch.__version__ is not a str. it's a version type that can correctly compare with str.

Parse the schema, to positional_num and keyword_list, and detect if the argument should be specially treated.
Cannot use 'torch._C.parse_schema' because 'torch._C.Argument' has no 'kwarg_only' on pytorch v1.8.x
Using a lexer-parser like method to parse it.
Re-write from torch/csrc/jit/frontend/function_schema_parser.cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

if this function a mirror rewrite from this cpp file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not totally equal to the original function in cpp. Actually, the original parser in that cpp file is too specific and too long (5k+ lines). I wrote a much simpler syntax analyzer, deleted some error check and specific type check to shorten the code.

@Louis-J
Copy link
Contributor Author

Louis-J commented Oct 18, 2022

/azp run full test - compression

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@Louis-J
Copy link
Contributor Author

Louis-J commented Oct 18, 2022

/azp run full test - nas

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@Louis-J
Copy link
Contributor Author

Louis-J commented Oct 18, 2022

/azp run fast test

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@J-shang J-shang merged commit c53be96 into microsoft:master Oct 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants