Skip to content
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.

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

[FxImporter] Add backend lowering to Fx API #3205

Closed
wants to merge 3 commits into from

Conversation

penguin-wwy
Copy link
Collaborator

  • Extract lower_mlir_module to a common file
  • Lower torch module to backends in Fx API

@penguin-wwy penguin-wwy force-pushed the fx_dev branch 2 times, most recently from eb7b1a9 to 1947d12 Compare April 23, 2024 07:16
@penguin-wwy
Copy link
Collaborator Author

I think we need to provide the same functionalities in FxImporter as PT1, which can help facilitate the migration from PT1 to Fx. @stellaraccident

@penguin-wwy penguin-wwy changed the title [FxImporter] Add backend lowering to Fx API. [FxImporter] Add backend lowering to Fx API Apr 24, 2024
@stellaraccident
Copy link
Collaborator

I think we need to provide the same functionalities in FxImporter as PT1, which can help facilitate the migration from PT1 to Fx. @stellaraccident

We can do that, but we can't have the dependency go this way. Question of how the code is layered. Things in the PT1 directory can depend on things outside of it, but not the other way.

@penguin-wwy
Copy link
Collaborator Author

I think we need to provide the same functionalities in FxImporter as PT1, which can help facilitate the migration from PT1 to Fx. @stellaraccident

We can do that, but we can't have the dependency go this way. Question of how the code is layered. Things in the PT1 directory can depend on things outside of it, but not the other way.

So I should migrate the common code from pt1 to the python directory, right?

Another question is when should we completely merge the contents of pt1 into the python directory? The current situation is causing a lot of inconvenience.

@stellaraccident
Copy link
Collaborator

I think we need to provide the same functionalities in FxImporter as PT1, which can help facilitate the migration from PT1 to Fx. @stellaraccident

We can do that, but we can't have the dependency go this way. Question of how the code is layered. Things in the PT1 directory can depend on things outside of it, but not the other way.

So I should migrate the common code from pt1 to the python directory, right?

Another question is when should we completely merge the contents of pt1 into the python directory? The current situation is causing a lot of inconvenience.

What is in the PT1 directory has either a direct or indirect dependency on the JitIR importer, and that is the problem. That is the piece that cannot move to the new world. I tried when starting out to take it apart in one step, but it was not feasible then because we didn't have a replacement for it yet (so the practical motivation to drive the change wasn't there).

Now that we have the new lower level replacement for that (FxImporter), we can peel the Python APIs in the pt1 dir back to the root. The rubric is that the new parts of the project should work with -DTORCH_MLIR_ENABLE_JIT_IR_IMPORTER=OFF.

If we're reworking the high level APIs, can we create a python/api/... package, put the user level API stuff in there, and then have the root python/init.py import the public API things users should use (but not just have everything inlined there).

So yes, if you're up for it, you can start moving support code out of pt1, but it will need to be reworked as it goes to not have a dependency on things tied to the JitIR importer (which has a hard dependency on PyTorch's C++ API).

@penguin-wwy
Copy link
Collaborator Author

I think we need to provide the same functionalities in FxImporter as PT1, which can help facilitate the migration from PT1 to Fx. @stellaraccident

We can do that, but we can't have the dependency go this way. Question of how the code is layered. Things in the PT1 directory can depend on things outside of it, but not the other way.

So I should migrate the common code from pt1 to the python directory, right?
Another question is when should we completely merge the contents of pt1 into the python directory? The current situation is causing a lot of inconvenience.

What is in the PT1 directory has either a direct or indirect dependency on the JitIR importer, and that is the problem. That is the piece that cannot move to the new world. I tried when starting out to take it apart in one step, but it was not feasible then because we didn't have a replacement for it yet (so the practical motivation to drive the change wasn't there).

Now that we have the new lower level replacement for that (FxImporter), we can peel the Python APIs in the pt1 dir back to the root. The rubric is that the new parts of the project should work with -DTORCH_MLIR_ENABLE_JIT_IR_IMPORTER=OFF.

If we're reworking the high level APIs, can we create a python/api/... package, put the user level API stuff in there, and then have the root python/init.py import the public API things users should use (but not just have everything inlined there).

So yes, if you're up for it, you can start moving support code out of pt1, but it will need to be reworked as it goes to not have a dependency on things tied to the JitIR importer (which has a hard dependency on PyTorch's C++ API).

Thank you for the guidance, I think I've understood where the core issue lies.

I need to break down the content of this PR. First, prioritize decomposing and extracting the common parts to the Python directory, then assemble the new user API. This PR will be closed, and it will be replaced by the PRs resulting from the breakdown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants