Restructuring demos #531
Closed
Restructuring demos #531
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for resplendent-gnome-14e531 canceled.
|
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
a6428e0 to
1586dd6
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
1586dd6 to
2a6bdf2
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
2a6bdf2 to
889d990
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
889d990 to
29f8335
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
29f8335 to
5990a94
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
5990a94 to
5db3e60
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
5db3e60 to
8bd33d4
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
8bd33d4 to
b03e9e1
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
b03e9e1 to
026c344
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
026c344 to
972ec0d
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
972ec0d to
bcfdac7
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
bcfdac7 to
b38515a
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
b38515a to
63ef6e1
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
63ef6e1 to
012d4b8
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
012d4b8 to
5c4fa28
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
5c4fa28 to
5b2ab55
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
5b2ab55 to
20901be
Compare
Summary: Pull Request resolved: pytorch/executorch#531 ----- ## Confusion arising from the current code structure: Reference to current code structure: https://github.com/pytorch/executorch/tree/main/examples The current code structure under `executorch/examples` mixes different levels of detail. For instance, the directories `backend/`, `quantization/`, and `export/` are intended for component-level demo code, such as showcasing quantization/export/delegation workflows or providing sample backend/quantizer/partitioner implementations. Assembling them to create a AOT path should not belong to any of it. However, the current structure combines recipes and individual components: - All contents of `backend/` are exclusively tailored for [xnnpack](https://github.com/pytorch/executorch/tree/main/examples/backend). - [export+delegate+composite](https://github.com/pytorch/executorch/blob/main/examples/export/export_and_delegate.py) and [bundled program example](https://github.com/pytorch/executorch/blob/main/examples/export/export_bundled_program.py) are placed in `export/` - Different runtimes are placed haphazardly., with items like [executor_runner](https://github.com/pytorch/executorch/tree/main/examples/executor_runner), [bundled_executor_runner](https://github.com/pytorch/executorch/tree/main/examples/bundled_executor_runner), and [xnn_executor_runner](https://github.com/pytorch/executorch/tree/main/examples/backend) - Backend-specific demos are scattered in the top-level directory, such as [arm's demo](https://github.com/pytorch/executorch/tree/main/examples/arm) and [partitioner/quantizer demo](https://github.com/pytorch/executorch/tree/main/examples/example_quantizer_and_delegate) Issues that are not directly tie to code restructuring but will need to be addressed as well. **Non-functional code/demo** (fix or hide): - [Unit tests](https://github.com/pytorch/executorch/tree/main/examples/export/test) under the `export/` directory are not runnable in the OSS environment. - The [example_quantizer_and_delegate](https://github.com/pytorch/executorch/tree/main/examples/example_quantizer_and_delegate) is not functioning and should be rectified or hidden. - The [arm's demo](https://github.com/pytorch/executorch/tree/main/examples/arm) is not runnable directly w/o additional guide. ----- ## Proposed code structure: organize demos in a way that is easier for users to understand how pieces are put together to create an e2e experience. We can go with this "vertical" structure if we could come up and agree on guidelines of whether a new demo should be considered as "adding a new demo" or "extending an existing demo". The purpose is to prevent randomly dumping demos in the top-level executorch/examples folder with boilerplate code. While this doesn't violate the rule of structuring demos vertically for user understanding, , but it literally has no structure at all, and it will quickly grow of out our maintenance. In my opinion, it would make sense to consider the following cases as **"adding a new demo"**: 1. Demo of a new 1st/3rd party backend, e.g. portable (default backend), xnnpack, arm, coreml, etc. 2. Demo of toolings, e.g. selective build, productivity sdk, etc. 3. Target specific apps, e.g. playground apps for Android/iOS/Embedded The rests will be considered as **"extending an existing demo"**, for example: - Demo of composibitlity, e.g. delegate and composite, whole/partial graph lowering, custom ops - Demo of specific components, e.g. quantization workflow, 2-stage export flow, example quantizer/partitioner, etc. After PTC we will spend more efforts on expanding and polishing the two major 1st-party demos (`examples/portable/` & `examples/xnnpack/`) we own in the long run. Maybe `examples/sdk` as well. So with the guidelines, the new top-level structure will looks like: ``` executorch/examples/ ├── README.md # top-level description for demos and folder structures ├── models/ ├── xnnpack/ # 1p e2e ├── portable/ # 1p e2e ├── arm/ # 3p backend e2e ├── qualcomm/ # 3p backend e2e ├── apple/ # 3p backend e2e ├── third-party/ # for the short-term, we can assume a centralized place for all third-party libs required by demos, e.g. sam, llama, etc. ├── selective_build/ ├── sdk/ ├── demo-apps/ └── utils/ # future ``` If we zoom in to `examples/portable/`, it will look like: ``` executorch/examples/portable/ ├── executor_runner/ ├── bundled_executor_runner/ ├── custom_ops/ ├── scripts/ │ ├── export_and_delegate.py │ ├── export_bundled_program.py │ └── export.py ├── test/ ├── __init__.py ├── utils.py └── README.md ``` If we zoom in to `examples/xnnpack/`, it will look like: ``` executorch/examples/xnnpack/ ├── quantization/ │ ├── example.py │ ├── TARGETS │ ├── test_quantize.sh │ └── utils.py ├── __init__.py ├── aot_compiler.py └── README.md ``` Reviewed By: mergennachin Differential Revision: D49714823 fbshipit-source-id: 72c28d9f176b3efae9f1e21bb2ff06f3dfff0fce
|
This pull request was exported from Phabricator. Differential Revision: D49714823 |
guangy10
force-pushed
the
export-D49714823
branch
from
20901be to
d4c665a
Compare
|
This pull request has been merged in 85ba2b7. |
Gasoonjia
pushed a commit
that referenced
this pull request
Jul 30, 2024
* runner-aoti on cuda * transfer results back to CPU * transfer results back to CPU * runner-aoti on cuda
Gasoonjia
pushed a commit
that referenced
this pull request
Jul 30, 2024
* make --device fast the default * Update iOS.md (#517) * Update iOS.md * Update iOS.md * Pip to pip3 (#504) * remove macos-12 test * pip to pip3 * break aoti CI jobs separately (#500) * init * fixes * more fixes * fixes * fix * fix * bug fix * add objcopy update * suppress int8 * undefined variable --------- Co-authored-by: Michael Gschwind <[email protected]> * Support llama3 in chat in run.cpp (#486) * refactor chat runner in preparation for llama3 * add sketch for llama3 prompt template and move to returning tokens * fix tiktoken * fixes to chat * add default llama_ver * Add tests for quantize json, add cuda device specification and precision to cuda.json (#519) * remove code for no KV Cache path (#527) * Update ADVANCED-USERS.md (#529) Update Advanced Users description to reflect changes in the repo since the description was initially created. * runner-aoti on cuda (#531) * runner-aoti on cuda * transfer results back to CPU * transfer results back to CPU * runner-aoti on cuda * Update runner_build.md (#530) Update description of runner and build process in runner_build.md * clean up runner code a little (#532) * clean up runner code a little * update * update * pull out generate loop in chat * updates * edit docs * typo * move int8 linear class and function into qops.py (#534) * add dtype tests for runner-aoti + runner-et (#539) * add dtype tests for runner-aoti + runner-et * typo * Quantized embedding (#536) * move int8 linear class and function into qops.py * move Quantized Embedding to qops.py * Move Linear int4 to qops (#537) * move int8 linear class and function into qops.py * move Quantized Embedding to qops.py * move int4 linear to qops * Revert "add dtype tests for runner-aoti + runner-et (#539)" (#548) This reverts commit a7a24577a65be67ac9ae4dc05452f35d9c49e5d1. * fix generate for llama3 (#538) * fix generate for llama3 * switch more things to C * remove C++ header * add delegation visualization instructions (#551) * Add dtype runner aoti (#552) * add dtype tests for runner-aoti + runner-et * typo * add dtype test runner-aoti * test sdpa with fp16 (#553) * test sdpa with fp16 * kv cache fp32 * typo * update (#560) * Only support newest versions of lm-eval (#556) Summary: remove support for lm-eval 0.3 to reduce the options we have Test Plan: CI Reviewers: Subscribers: Tasks: Tags: * split cpu eval CI by dtype (#554) * split cpu eval CI by dtype * fix * differentiate names with checks * keep one name the same as old * fix * Removing duplicate HF issue message from README (#559) Co-authored-by: Michael Gschwind <[email protected]> * doc updates (#567) * Add VM-safe MPS check --------- Co-authored-by: Anthony Shoumikhin <[email protected]> Co-authored-by: metascroy <[email protected]> Co-authored-by: Nikita Shulga <[email protected]> Co-authored-by: lucylq <[email protected]> Co-authored-by: Jerry Zhang <[email protected]> Co-authored-by: Jack-Khuu <[email protected]>
Gasoonjia
pushed a commit
that referenced
this pull request
Jul 30, 2024
* code beautification * code beautification, move functions together * make --device fast the default (#515) * make --device fast the default * Update iOS.md (#517) * Update iOS.md * Update iOS.md * Pip to pip3 (#504) * remove macos-12 test * pip to pip3 * break aoti CI jobs separately (#500) * init * fixes * more fixes * fixes * fix * fix * bug fix * add objcopy update * suppress int8 * undefined variable --------- Co-authored-by: Michael Gschwind <[email protected]> * Support llama3 in chat in run.cpp (#486) * refactor chat runner in preparation for llama3 * add sketch for llama3 prompt template and move to returning tokens * fix tiktoken * fixes to chat * add default llama_ver * Add tests for quantize json, add cuda device specification and precision to cuda.json (#519) * remove code for no KV Cache path (#527) * Update ADVANCED-USERS.md (#529) Update Advanced Users description to reflect changes in the repo since the description was initially created. * runner-aoti on cuda (#531) * runner-aoti on cuda * transfer results back to CPU * transfer results back to CPU * runner-aoti on cuda * Update runner_build.md (#530) Update description of runner and build process in runner_build.md * clean up runner code a little (#532) * clean up runner code a little * update * update * pull out generate loop in chat * updates * edit docs * typo * move int8 linear class and function into qops.py (#534) * add dtype tests for runner-aoti + runner-et (#539) * add dtype tests for runner-aoti + runner-et * typo * Quantized embedding (#536) * move int8 linear class and function into qops.py * move Quantized Embedding to qops.py * Move Linear int4 to qops (#537) * move int8 linear class and function into qops.py * move Quantized Embedding to qops.py * move int4 linear to qops * Revert "add dtype tests for runner-aoti + runner-et (#539)" (#548) This reverts commit a7a24577a65be67ac9ae4dc05452f35d9c49e5d1. * fix generate for llama3 (#538) * fix generate for llama3 * switch more things to C * remove C++ header * add delegation visualization instructions (#551) * Add dtype runner aoti (#552) * add dtype tests for runner-aoti + runner-et * typo * add dtype test runner-aoti * test sdpa with fp16 (#553) * test sdpa with fp16 * kv cache fp32 * typo * update (#560) * Only support newest versions of lm-eval (#556) Summary: remove support for lm-eval 0.3 to reduce the options we have Test Plan: CI Reviewers: Subscribers: Tasks: Tags: * split cpu eval CI by dtype (#554) * split cpu eval CI by dtype * fix * differentiate names with checks * keep one name the same as old * fix * Removing duplicate HF issue message from README (#559) Co-authored-by: Michael Gschwind <[email protected]> * doc updates (#567) * Add VM-safe MPS check --------- Co-authored-by: Anthony Shoumikhin <[email protected]> Co-authored-by: metascroy <[email protected]> Co-authored-by: Nikita Shulga <[email protected]> Co-authored-by: lucylq <[email protected]> Co-authored-by: Jerry Zhang <[email protected]> Co-authored-by: Jack-Khuu <[email protected]> * add unpacking support (#525) * add unpacking support * fix typos and linter * perform parallel prefill when possible (#568) * perform parallel prefill when possible * typo * disable hack * remove print * remove debug messages which prevent export * fixes * stream results in generate.py (#571) * remove logging interfering with export --------- Co-authored-by: Anthony Shoumikhin <[email protected]> Co-authored-by: metascroy <[email protected]> Co-authored-by: Nikita Shulga <[email protected]> Co-authored-by: lucylq <[email protected]> Co-authored-by: Jerry Zhang <[email protected]> Co-authored-by: Jack-Khuu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Differential Revision: D49714823