Refine LLM getting started guide for uniformity, fix critical errors#2911
Refine LLM getting started guide for uniformity, fix critical errors#2911GregoryComer wants to merge 1 commit intopytorch:mainfrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/2911
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit 678ba94 with merge base dc7e4d5 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
ae6fe1a to
b9cd8ff
Compare
|
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
b9cd8ff to
8c1bd79
Compare
| if [[ -z $PYTHON_EXECUTABLE ]]; | ||
| then | ||
| if [[ -z $CONDA_DEFAULT_ENV ]] || [[ $CONDA_DEFAULT_ENV == "base" ]]; | ||
| if [[ -z $CONDA_DEFAULT_ENV ]] || [[ $CONDA_DEFAULT_ENV == "base" ]] || [[ ! -x "$(command -v python)" ]]; |
There was a problem hiding this comment.
Not sure how this happened, but heard of a case where python wasn't on the system path while in a conda env. Something to do with env/shell conflicts on mac? This change checks to make sure python actually exists. If not, default to python3, which seems to be the sanest option.
| PIP_EXECUTABLE=pip | ||
| else | ||
| PIP_EXECUTABLE=pip3 | ||
| fi |
There was a problem hiding this comment.
On mac when not under conda, pip may not exist. We should use pip3 when PYTHON_EXECUTABLE=python3.
|
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
byjlw
left a comment
There was a problem hiding this comment.
I added a couple of comments, but overall this is way better and feel free to merge
|
|
||
| ## Prerequisites | ||
|
|
||
| Let’s start by getting an ExecuTorch environment: |
There was a problem hiding this comment.
this is better. I like not having external links for instructions.
| 2. If you’re new to ExecuTorch follow [these steps](https://pytorch.org/executorch/main/getting-started-setup.html#set-up-your-environment) to set up your environment. | ||
| ::::{tab-set} | ||
| :::{tab-item} conda | ||
| Instructions on installing miniconda can be [found here](https://docs.anaconda.com/free/miniconda). |
There was a problem hiding this comment.
can we eliminate the need for Conda.
Can you re-write this so that it uses env?
There was a problem hiding this comment.
O looks like you have both options here. Perfect. I'd change the instructions just a tad so it calls out two different options up front before giving the conda instructions
| ``` | ||
| string prompt = "Hello world!"; | ||
| ``` | ||
| // Sample the next token from the logits. |
| ``` | ||
|
|
||
| Also ExecuTorch provides different backend support for mobile acceleration. Simply call `to_backend()` with the specific backend partitioner on edge_manager during exportation. Take Xnnpack delegation as an example: | ||
| To export, run the script with `python export.py` (or python3, as appropriate for your environment). It will generate a `nanogpt.pte` file in the current directory. |
There was a problem hiding this comment.
Should be export_nanogpt.py?
| et_program = edge_manager.to_executorch() | ||
| ``` | ||
| ```cpp | ||
| // main.cpp |
There was a problem hiding this comment.
Do we plan to put them to the same piece?
| Finally, ensure that the runner links against the `xnnpack_backend` target in CMakeLists.txt. | ||
|
|
||
| ``` | ||
| add_executable(nanogpt_runner nanogpt_runner.cpp) |
There was a problem hiding this comment.
Thanks. I updated the text here to be more clear.
| TODO: This is a placeholder. Finish. | ||
| ``` | ||
| vector<int64_t> outputs = generate(llm_model, tokens, sampler, /*target_output_length*/20); | ||
| https://github.com/GregoryComer/et-tutorials/blob/quantization/nanogpt/managed_tensor.h |
There was a problem hiding this comment.
| @@ -348,15 +487,19 @@ target_link_libraries( | |||
| xnnpack_backend) # Link the XNNPACK backend | |||
There was a problem hiding this comment.
Let's tell user exactly what's the diff?
There was a problem hiding this comment.
Also etdump is causing issue?
| The `get_delegation_info()` method provides a summary of what happened to the model after the `to_backend()` call: | ||
|
|
||
| ```python | ||
| from executorch.exir.backend.utils import get_delegation_info |
There was a problem hiding this comment.
Good catch. I'll update.
9f75846 to
6996c48
Compare
|
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
6996c48 to
6ba87b4
Compare
|
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
6ba87b4 to
678ba94
Compare
|
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
…ytorch#2911) Summary: Update the LLM getting started guide for uniform tone and tense. Informally following the Google developer documentation style guide: https://developers.google.com/style. Also, resolve a number of outstanding issues with incorrect or misleading documentation and steps. For reference, here are links to the current and proposed LLM guide: https://docs-preview.pytorch.org/pytorch/executorch/2911/llm/getting-started.html (proposed) https://pytorch.org/executorch/main/llm/getting-started.html (live) Pull Request resolved: pytorch#2911 Reviewed By: Gasoonjia, byjlw Differential Revision: D55867181 Pulled By: GregoryComer
|
@GregoryComer merged this pull request in 01bac3d. |
|
@pytorchbot cherry-pick --onto release/0.2 -c docs |
…2911) Summary: Update the LLM getting started guide for uniform tone and tense. Informally following the Google developer documentation style guide: https://developers.google.com/style. Also, resolve a number of outstanding issues with incorrect or misleading documentation and steps. For reference, here are links to the current and proposed LLM guide: https://docs-preview.pytorch.org/pytorch/executorch/2911/llm/getting-started.html (proposed) https://pytorch.org/executorch/main/llm/getting-started.html (live) Pull Request resolved: #2911 Reviewed By: Gasoonjia, byjlw Differential Revision: D55867181 Pulled By: GregoryComer fbshipit-source-id: 5e865eaa4a0ae52845963b15c221a3d272431448 (cherry picked from commit 01bac3d)
Cherry picking #2911The cherry pick PR is at #2939 Details for Dev Infra teamRaised by workflow job |
…2911) Summary: Update the LLM getting started guide for uniform tone and tense. Informally following the Google developer documentation style guide: https://developers.google.com/style. Also, resolve a number of outstanding issues with incorrect or misleading documentation and steps. For reference, here are links to the current and proposed LLM guide: https://docs-preview.pytorch.org/pytorch/executorch/2911/llm/getting-started.html (proposed) https://pytorch.org/executorch/main/llm/getting-started.html (live) Pull Request resolved: #2911 Reviewed By: Gasoonjia, byjlw Differential Revision: D55867181 Pulled By: GregoryComer fbshipit-source-id: 5e865eaa4a0ae52845963b15c221a3d272431448 (cherry picked from commit 01bac3d)
Update the LLM getting started guide for uniform tone and tense. Informally following the Google developer documentation style guide: https://developers.google.com/style. Also, resolve a number of outstanding issues with incorrect or misleading documentation and steps.
For reference, here are links to the current and proposed LLM guide:
https://docs-preview.pytorch.org/pytorch/executorch/2911/llm/getting-started.html (proposed)
https://pytorch.org/executorch/main/llm/getting-started.html (live)