[rascani]

The component's README is its page on the ESP Component Registry, and it
opened with how the tree is generated and versioned, which tells a
newcomer nothing about what ExecuTorch is or where to start. It now
introduces ExecuTorch and how a model goes from torch.export to a .pte
that the runtime runs from flash, lists what the component carries, and
gives the command that creates the hello_executorch project from the
registry. The usage notes become steps in the order a project takes
them, with an export example for the ESP-NN delegate, run against the
current package, and the et_model example from its header. How the tree
is generated and versioned moves to the end.

Authored with Claude Code.

cc @psiddh @AdrianLundell @digantdesai

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24861

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• [Mitigated] PR CI jobs created 12:15–12:55 PM PT on 11/11 stuck in Queued (GitHub Actions incident): push again to restart

✅ You can merge normally! (2 Unrelated Failures)

As of commit b1426f3 with merge base a73396e ():

FLAKY - The following job failed but was likely due to flakiness present on trunk:

• Test WebGPU Backend / test-webgpu / test-backend-linux (webgpu, operators) / linux-job (gh) (similar failure)
  curl: (22) The requested URL returned error:

BROKEN TRUNK - The following job failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

• Test WebGPU Backend / test-webgpu / test-backend-linux (webgpu, models) / linux-job (gh) (trunk failure)
  curl: (22) The requested URL returned error:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[rascani]

@claude review

[claude]

Claude finished @rascani's task in 2m 52s —— View job

---

The rewrite holds up against the code, but it drops the esp_nn:: op library the component still ships, and a few claims go further than the code supports (memory planning, hello_executorch availability, RC create-project).

Detailed review

PR Review: #24861

Summary

The PR reorganizes the ESP-IDF component's registry README around what ExecuTorch is, a quick start, and step-by-step usage. Three reviewers checked every changed line against build_idf_component.sh, et_model.h, backends/espressif, and the publish workflow. One finding is a real omission. The rest are minor accuracy gaps. The Python export example, the C API names, the CMake/Kconfig names, the version scheme, the @ET_COMPONENT_VERSION@ substitution and the links all match the code.

Code Quality

• README.md:38-39: the esp_nn:: op library is gone (all 3 reviewers). The old text listed four kernel libraries: "portable, quantized, the Cadence backend's generic kernels and the esp_nn:: op library". The new bullet lists only three, and the op library is not mentioned anywhere else (git diff shows the old line removed, -and the \esp_nn::` op library). The component still ships it: build_idf_component.sh:39-40,500addsesp_nn backends/espressif/ops/operators.yaml, and CI exercises it (.ci/scripts/test_espressif_idf_component.sh). The "Export a model" section also covers only the delegate path. It never mentions to_espnn_edge(..., use_operator_library=True) (backends/espressif/lowering.py), which emits esp_nn::` operators instead of delegates. Please restore the library to the list, and add a sentence on the operator-library path.

• README.md:22-24: the memory-planning claim goes too far. The README says lowering "plans where every tensor lives in memory, so the device runs the model in arenas whose sizes are known before it starts". et_model.h:62-71 says otherwise:

  • The method pool holds "non-planned tensors" and defaults to a guessed 64 KiB.
  • With no temp pool (the default), "the runtime then mallocs scratch through the PAL during execute".

  Suggest "plans where the model's activations live", and drop the claim that all sizes are known up front.

• README.md:61-63 (and :47): a self-generated copy may not contain examples/hello_executorch. build_idf_component.sh:290-292 sets EXAMPLE=false and only prints a warning under --allow-dev-stamp when the installed executorch doesn't match the checkout. That is the common local-dev case, where the README sends the user to "build examples/hello_executorch in place". A short qualifier would fix it, for example "if the generator included it (it needs an executorch package built from the same checkout)".

• README.md:55: the create-project command fails on RC pages (lower confidence, not run). publish-espressif-idf.yml:82-83,108-110 publishes -rc.N versions to the staging registry only, and the README is rendered per version. An RC's page therefore shows pytorch/executorch==X.Y.Z-rc.N:hello_executorch. That command resolves against the production registry by default, where the version doesn't exist. A note that RC users must point idf.py at the staging registry would help. The == dependency snippet has the same gap, but that predates this PR.

• README.md:179-195: the "Run the model" snippet.

  • model_pte/model_pte_size appear from nowhere. The pointer to hello_executorch/main/CMakeLists.txt doesn't say that the example uses a hand-written .S with .balign 16 + .incbin. Plain EMBED_FILES doesn't guarantee that alignment, and et_model_create rejects misaligned data (src/et_model.cpp:233-237). One sentence naming the technique would stop readers reaching for EMBED_FILES.
  • The snippet ignores the bool returns of et_model_set_input and et_model_execute (et_model.h:160,166). It is copied from the header comment, but readers will copy it from the registry page. Checking them as the snippet already does for et_model_load_method costs one line.

Recommendation

Approve once the esp_nn:: op library is back in the "What the component provides" list. The remaining items are minor wording fixes that can go into this PR or a follow-up.

[rascani]

@claude review

[claude]

Claude finished @rascani's task in 3m 33s —— View job

---

The amended README fixes most of what the last review raised, but the esp_nn:: op library is still missing from the component's contents, and the note on when the generator includes hello_executorch doesn't match build_idf_component.sh.

Detailed review

PR Review: #24861

Summary

This PR rewrites the ESP-IDF component's registry README as an intro to ExecuTorch, a quick start, and usage steps in order. Three independent reviewers checked it against the code, and I fact-checked their findings. The Python export example, the et_model API, ET_COMPONENT_SMOKE: PASS, the .balign 16/.incbin note, the version scheme and the @...@ placeholders all match the code. Two minor accuracy problems remain, plus a few smaller notes.

Code Quality

• README.md:39-40: the esp_nn:: op library is still missing (all 3 reviewers). The kernel-library bullet lists portable, quantized and Cadence generic only. The component still ships the ESP-NN operator library:

  • build_idf_component.sh:40 names it in the generator's header comment, and build_idf_component.sh:503 ships it ("esp_nn backends/espressif/ops/operators.yaml").
  • to_espnn_edge(..., use_operator_library=True) (backends/espressif/lowering.py) produces .ptes that need it.

  "Export a model" also describes only the delegate path. Please add the library to the bullet, and add one sentence on use_operator_library=True to the export section. Fix this →

• README.md:65-68: the rule for when the generator includes hello_executorch is wrong (2 reviewers, confirmed). The README says the generator includes it "when it runs with the executorch package built from the same commit". The code differs in two ways:

  • versions_match (tools/gen_kernels.py:75-85) also accepts the X.Y.Z / X.Y.Z+cpu release wheel for a release or ~N respin.
  • When nothing matches, the generator leaves the example out only under --allow-dev-stamp. Every other stamp exits with an error (build_idf_component.sh:293-299).

  Suggested wording: "unless it was a --allow-dev-stamp build without an executorch package built from that commit". The "What the component provides" bullet at line 48 lists examples/hello_executorch with no such qualifier.

• README.md:62-65: the RC registry variable needs to stay set for the build too (2 reviewers, low confidence, not run). hello_executorch/main/idf_component.yml.in has no registry_url, and the registry drops its override_path. So idf.py build resolves ==X.Y.Z-rc.N again and needs IDF_COMPONENT_REGISTRY_URL as well. If a reader sets it inline only for create-project-from-example, the build then can't find the RC. Saying "export it in the shell" would avoid this, as would pointing to the registry_url note at lines 81-82.

• README.md:196-201: the C snippet's error branches fall through (minor). After a failure it still calls et_model_output_data. It also never checks whether that call returned NULL (et_model.h:168-171). The snippet mirrors the header comment, but readers will copy it from the registry page, so it should return from the error branches.

• README.md:121,127: undefined placeholders in the export example (nit). MyModel and calibration_inputs are undefined, so pasting the block raises a NameError. report is assigned but never explained, and it is the list of CPU-fallback ops users will want to check. A short comment on each would help.

Recommendation

Approve once the esp_nn:: op library is back in the list and the generator caveat is corrected. The other items are optional wording fixes.