Skip to content

Qualcomm AI Engine Direct - Add QNN/HTP joint-graph training toy example - #24205

Open
likhitanavali wants to merge 1 commit into
pytorch:mainfrom
CodeLinaro:dev1/lnavali/nsp-training
Open

likhitanavali wants to merge 1 commit into
pytorch:mainfrom
CodeLinaro:dev1/lnavali/nsp-training

Conversation

@likhitanavali

Copy link
Copy Markdown

Summary

Add a minimal, self-contained QNN/HTP joint-graph training example under examples/qualcomm/training/. A 4-layer bias-free toy model is exported as a joint forward+backward graph via aot_export_joint_with_descriptors, lowered to the QNN delegate, and exercised through a full train → inference pipeline using real Hexagon hardware (C++ qnn_toy_model_trainer / qnn_toy_model_inference). Trained weights are saved as trained_model.safetensors and consumed by the inference runners. A run_toy_model.py script handles push, training, and inference to a connected Android device in one command.
A clean baseline for QNN/HTP training bring-up.

Release notes: examples: Add minimal QNN/HTP on-device training example (examples/qualcomm/training/): exports a toy model as a joint forward+backward graph, trains on real Hexagon/HTP hardware via C++ runners and runs inference from the trained weights.

Test plan

Tested full train → inference pipeline on SM8850 device using examples/qualcomm/training/run_toy_model.py

@pytorch-bot

pytorch-bot Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit 17bc99a with merge base 3ef59e9 (image):

NEW FAILURE - The following job has failed:

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

@meta-cla

meta-cla Bot commented Oct 27, 2026

Copy link
Copy Markdown

Hi @likhitanavali!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@linux-foundation-easycla

linux-foundation-easycla Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: likhitanavali / name: lnavali (17bc99a)

@likhitanavali

Copy link
Copy Markdown
Author

@pytorchbot label "release notes: qualcomm"

@pytorch-bot pytorch-bot Bot added the release notes: qualcomm Changes to the Qualcomm backend delegate label Oct 27, 2026
@psiddh

psiddh commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

@claude review this pr

@claude

claude Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Claude finished @psiddh's task in 3m 41s —— View job


This PR adds a QNN/HTP joint-graph training toy example, but four problems need fixing before merge: --config never reaches the device, the documented --weight_sharing true command fails, the stock build.sh never builds the binaries, and --skip_train deletes the trained weights unless push is also skipped.

Detailed review

PR Review: #24205

Summary

This PR adds a self-contained example under examples/qualcomm/training/toy_model/. It exports a toy model as a joint forward+backward graph, lowers it to QNN/HTP, and trains and runs inference on-device with custom C++ runners. Several user-facing paths do not work as documented, and nothing in CI tests the example.

Code Quality

  • run_toy_model.py:160: --config is ignored on the device. artifact_files always pushes os.path.join(script_dir, "config.json"), not args.config. qnn_toy_model_trainer and qnn_toy_model_inference read steps, lr, seed and log_every from that pushed file. With --config my.json containing "steps": 50, the host uses my.json to pick the artifact dir, but the device still trains with the defaults. The docstring at line 25 says all fields come from --config. Fix this →
  • run_toy_model.py:85 with trainer.cpp:131: inference-only runs delete the trained weights. The trainer writes trained_model.safetensors into artifacts/. Unless --skip_push is set, push_artifacts() first runs rm -rf {art_dir} on that directory. The --skip_train help text says "run inference only", so --skip_train without --skip_push wipes the weights, and inference then aborts on the missing file. Either make --skip_train imply skipping the artifacts push, or keep trained_model.safetensors out of the directory that gets wiped. The trained weights are also never pulled back to the host, although the PR description says they are "saved".
  • run_toy_model.py:150: soc_model from config.json is ignored. QnnConfig.load_config(args) only sees CLI args, while export_toy_model.py defaults soc_model from config.json. Leaving out --soc_model hits the QnnConfig assert, even though the config has a value. Passing a SoC that differs from the one used at export pushes HTP libraries for the wrong arch, and nothing detects the mismatch. Other examples also honor --config_file (QnnConfig.load_config(args.config_file if args.config_file else args)); this script adds a second --config alongside it. Reusing a single config path would avoid that.
  • export_toy_model.py:53 vs :129-134: weight-to-parameter mapping can shift (latent). _make_fwd_wrapper zips all toy_model.named_parameters() names with weights. weights is filtered down to the primals_N placeholders left after unused-placeholder removal. If a model change leaves one parameter unused, the forward .pte silently binds weights to the wrong layers. The forward signature then also stops matching manifest.param_names. The current ToyModel does not trigger this. Build the wrapper from the same filtered param_names list.
  • trainer.cpp:87-95: no check that each gradient matches its weight. The SGD loop reads weights[i]->numel() floats from outputs->at(2 + i) without checking that grad numel and dtype match. A stale manifest.json, or an output order that differs from param_names, causes an out-of-bounds read instead of an error. Add an ET_CHECK_MSG on grad.numel() and grad.scalar_type().
  • common.cpp:35-40: unbounded safetensors header size. header_size comes straight from the file and is used to allocate a std::string with no limit. A truncated or corrupt file throws bad_alloc or length_error and aborts, instead of failing with an ET_CHECK_MSG. Check it against the file size first.
  • trainer.cpp:97, inference.cpp:58: division by zero. step % log_every divides by zero if config.json sets log_every: 0.
  • inference.cpp:43: inference does not verify training. It reuses the training steps value (default 500) as the batch count and logs pred[0] on new random inputs, with no target or comparison. That shows the forward method runs, but not that the trained weights are better than the initial ones. Logging loss against the training target on the training x before and after training would make the train → inference pipeline meaningful.

Infrastructure

  • examples/qualcomm/CMakeLists.txt:86-91: the standard build never produces the binaries. BUILD_QNN_TRAINING_EXAMPLE defaults to OFF. backends/qualcomm/scripts/build.sh neither sets it nor offers a way to pass extra CMake args. After a normal build.sh build, run_toy_model.py fails inside SimpleADB.push() with "Missing the following libraries", which does not point at the cause. The README (lines 59-62) only says to "pass -DBUILD_QNN_TRAINING_EXAMPLE=ON when configuring". Either add a build.sh flag or give the exact configure/build command in the README.
  • export/export_toy_model.py:35-45, run_toy_model.py:39-45: imports only work when run from the script's directory. The bare from config/joint_graph/model import ... imports and the sys.path.insert fallback depend on the working directory. Other examples/qualcomm scripts use absolute executorch.examples.qualcomm... imports and run with python -m. config is also a module name that something else can easily shadow.

Testing

  • No test or CI coverage. The PR adds no TestExampleScript entry in backends/qualcomm/tests/test_qnn_delegate.py and no host-side test of the export. This matters because export/joint_graph.py depends on private torch.fx internals (_PyTreeCodeGen, _PyTreeInfo, graph._codegen, "verified against PyTorch 2.13"). When those change, the except (AttributeError, TypeError) only logs a warning, and the failure shows up later as an unclear leaf-count mismatch. At a minimum, add an export-only test (compile to .pte, no device needed), and fail fast instead of only warning.

API Design

  • README.md:102: the documented flag syntax is wrong. The README says to use --weight_sharing true, but --weight_sharing uses argparse.BooleanOptionalAction, which takes no value. That command exits with "unrecognized arguments: true". The working forms are --weight_sharing and --no-weight_sharing.
  • README.md:108 vs run_toy_model.py:148: the device path doesn't match the docs. The README says artifacts go to /data/local/tmp/$(whoami)/..., but the code uses os.environ.get("USER", "user"). In CI containers, where USER is often unset, that becomes /data/local/tmp/user/. The path is also interpolated unquoted into adb shell strings (rm -rf {art_dir}).

Recommendation

Request Changes

These issues block merge:

  • --config is not used on the device.
  • The documented --weight_sharing true command fails.
  • The standard build.sh never builds the binaries.
  • The inference-only path deletes the trained weights.

Each is a small fix. Some CI coverage of the export path is also strongly recommended, given the dependence on private torch APIs.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: qualcomm Changes to the Qualcomm backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants