-
Notifications
You must be signed in to change notification settings - Fork 24.9k
[OpenReg] Add Develop Notes for Integrating New Backend into PyTorch #158644
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
base: gh/fffrog/124/base
Are you sure you want to change the base?
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/158644
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 5 PendingAs of commit e4874ad with merge base ff0d56d ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
As the title stated. To facilitate the integration of the new backend, we plan to publish a new development note that details all the key components, hoping to speed up the development of other accelerators. This PR is the beginning of this note, which we will gradually improve and keep in sync with OpenReg's code. ghstack-source-id: 6b8c629 Pull-Request-resolved: #158644
As the title stated. To facilitate the integration of the new backend, we plan to publish a new development note that details all the key components, hoping to speed up the development of other accelerators. This PR is the beginning of this note, which we will gradually improve and keep in sync with OpenReg's code. ghstack-source-id: 7986da4 Pull-Request-resolved: #158644
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for starting this!
I'm sure there is going to be a LOT of things to add there. But very exciting to see this first draft!
docs/source/conf.py
Outdated
@@ -66,6 +66,7 @@ | |||
"sphinx.ext.linkcode", | |||
"sphinxcontrib.mermaid", | |||
"sphinx_sitemap", | |||
'sphinx_tabs.tabs', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@svekars is it ok to use this new extension?
docs/source/conf.py
Outdated
@@ -82,6 +83,9 @@ | |||
] | |||
sitemap_url_scheme = "{link}" | |||
|
|||
# todo options | |||
todo_include_todos = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this going to show a lot of unrelated todos?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @albanD
Sorry I forgot to change this Pr to a draft, I will remove all the todo later.
But I do need some advice from you. I will submit a simple RFC in the next few days. Please help me review whether the functional module scope of the document is insufficient/exceeding, whether the overall organization is reasonable, and whether the document can be placed in developer notes, etc.
There is another thing I need to ask for your opinion on, that is, there is a lot of work on this document and the supporting OpenReg code, so several colleagues will participate to speed up the overall speed. For the convenience of later development, can it be merged according to the module function, rather than merging after the entire doc document is completed?
docs/source/notes/accelerator.md
Outdated
@@ -0,0 +1,274 @@ | |||
```{eval-rst} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to add your new file to the index. Otherwise it's hard to get to from the website
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, thank you.
docs/source/notes/accelerator.md
Outdated
|
||
::::{tabs} | ||
|
||
:::{tab} C++ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PyTorch is very flexible and extensible, and we can extend it in many ways at the Python and C++ layers. To make the documentation clearer and easier to understand, I tried to use shpinx-tab to organize the documentation and divide some specific modules into two parts: Python and C++;
So, can we fix the abnormal display problem of sphinx-tab dark theme in pytorch_sphinx_theme2?
Please let me know, if there are other better ways to organize the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes tab sounds like a nice way to do this.
Feel free to send a PR to the theme repo to get this fixed !
docs/source/notes/accelerator.md
Outdated
The PrivateUse1-based third-party device integration mechanism has become the official path for integrating new devices into PyTorch. Ensuring the usability of this mechanism is crucial for enriching the hardware ecosystem of PyTorch. | ||
|
||
To assist third-party device developers in efficiently integrating new backends, this article introduces in detail the integration methods for typical PyTorch modules using a modular approach. It is accompanied by a streamlined code implementation from the official [torch_openreg][OpenReg URL] backend to help developers quickly get started while avoiding common pitfalls. | ||
|
||
This document is suitable for the following readers: | ||
|
||
* Developers who wish to integrate accelerator backends into PyTorch; | ||
* Developers interested in the principles of typical PyTorch modules; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest adding a first section explaining what we mean by backend quickly, the fact that this is all about eager, and the different pieces to consider (c10 integration, op implementations, python binding, testing, higher level modules (dist, autocast, etc)) and how Accelerator/PrivateUse1 makes your life easier.
If you're curious, you can take some of this more general context from https://docs.google.com/presentation/d/1rk3S0jqAPPBchZYkEle50p3fhMXAYBKj38jrFOJwlco/edit?slide=id.p34#slide=id.p34
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, I will add something on it and will cc you when it`s ready.
As the title stated. To facilitate the integration of the new backend, we plan to publish a new development note that details all the key components, hoping to speed up the development of other accelerators. This PR is the beginning of this note, which we will gradually improve and keep in sync with OpenReg's code. ghstack-source-id: 9c3aee3 Pull-Request-resolved: #158644
As the title stated. To facilitate the integration of the new backend, we plan to publish a new development note that details all the key components, hoping to speed up the development of other accelerators. This PR is the beginning of this note, which we will gradually improve and keep in sync with OpenReg's code. ghstack-source-id: 9ea01fc Pull-Request-resolved: #158644
As the title stated. To facilitate the integration of the new backend, we plan to publish a new development note that details all the key components, hoping to speed up the development of other accelerators. This PR is the beginning of this note, which we will gradually improve and keep in sync with OpenReg's code. ghstack-source-id: 2e10e33 Pull-Request-resolved: #158644
As the title stated. To facilitate the integration of the new backend, we plan to publish a new development note that details all the key components, hoping to speed up the development of other accelerators. This PR is the beginning of this note, which we will gradually improve and keep in sync with OpenReg's code. ghstack-source-id: 275a5c1 Pull-Request-resolved: #158644
As the title stated. To facilitate the integration of the new backend, we plan to publish a new development note that details all the key components, hoping to speed up the development of other accelerators. This PR is the beginning of this note, which we will gradually improve and keep in sync with OpenReg's code. ghstack-source-id: 35f9292 Pull-Request-resolved: #158644
As the title stated. To facilitate the integration of the new backend, we plan to publish a new development note that details all the key components, hoping to speed up the development of other accelerators. This PR is the beginning of this note, which we will gradually improve and keep in sync with OpenReg's code. ghstack-source-id: 926cc15 Pull-Request-resolved: #158644
Hey, sorry to bother you @albanD. |
@albanD, can you help review and approve this PR into the trunk first, and then use this PR as a basis for subsequent PRs to submit documents and document-related code one by one, and split subsequent PRs into smaller PRs for easier review? |
As the title stated. To facilitate the integration of the new backend, we plan to publish a new development note that details all the key components, hoping to speed up the development of other accelerators. This PR is the beginning of this note, which we will gradually improve and keep in sync with OpenReg's code. ghstack-source-id: 6f415c8 Pull-Request-resolved: #158644
Stack from ghstack (oldest at bottom):
To facilitate the integration of the new backend, we plan to publish a new development note that details all the key components,
hoping to speed up the development of other accelerators.
This PR is the beginning of this note, and involve the part of registration of operators and we will gradually improve it and
keep in sync with OpenReg's code.