Export nemotron-3.5-asr-streaming-0.6b to QNN - #3741
Conversation
dosubot
Bot
added
the
size:XXL
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds a QNN export pipeline for the nemotron-3.5-asr-streaming-0.6b model, including a new Dockerfile update, config generator script, and GitHub Actions workflows for ONNX/QNN export and conversion. It refactors the existing chunk-based ONNX export to a per-chunk-size matrix flow, adds wrapper/test scripts for encoder-decoder-joiner ONNX export and validation, and updates the QNN transducer runtime to support prompt-index-based multilingual prompting with language-tag token filtering in the recognizer. ChangesNemotron 3.5 ASR Streaming QNN Export & Runtime
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as GitHub Actions Workflow
participant Wrapper as wrapper.py
participant ONNX as encoder/decoder/joiner.onnx
participant Model as OnlineNemoTransducerModelQnn
participant Recognizer as OnlineRecognizerNemoTransducerQnnImpl
Workflow->>Wrapper: run export (chunk_size_ms, model_id)
Wrapper->>ONNX: torch.onnx.export encoder/decoder/joiner
Workflow->>Model: qnn-onnx-converter + QNN SDK tools
Note over Model: loads context binaries, checks prompt_index input
Recognizer->>Recognizer: derive prompt_index from language option
Recognizer->>Model: RunEncoder(features, num_frames, states, prompt_index)
Model->>Model: set "prompt_index" QNN input if supported
Model-->>Recognizer: encoder_out (transposed if needed)
Recognizer->>Recognizer: GetResult -> ContainsLanguageTag?
alt language tag present
Recognizer->>Recognizer: FilterLanguageTags(decoder_result)
end
Recognizer->>Recognizer: ConvertResult -> final text
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for exporting and running the Nemotron 3.5 ASR streaming model using Qualcomm QNN. The changes include Dockerfile updates, Python scripts for ONNX model export and testing, and C++ updates to handle language prompt IDs, model-specific transpositions, and language tag filtering. The review feedback highlights three key issues: a required cast of prompt_index to torch.int64 in wrapper.py to prevent a PyTorch ONNX export tracing error, the need to declare ANDROID_NDK_VERSION earlier in the Dockerfile before it is referenced in a LABEL instruction, and a missing defensive check in online-nemo-transducer-model-qnn.cc to avoid a potential division-by-zero crash.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| prompt.scatter_( | ||
| 2, | ||
| prompt_index.view(batch_size, 1, 1).expand(-1, time_steps, -1), | ||
| 1.0, | ||
| ) |
There was a problem hiding this comment.
In PyTorch, the scatter_ operation requires the index tensor to be of type torch.int64 (or torch.long). Since prompt_index is passed as torch.int32 (declared on line 373), calling prompt.scatter_ directly will raise a RuntimeError: index elements must be long during ONNX export tracing. Casting prompt_index to torch.int64 before scattering resolves this issue.
| prompt.scatter_( | |
| 2, | |
| prompt_index.view(batch_size, 1, 1).expand(-1, time_steps, -1), | |
| 1.0, | |
| ) | |
| prompt.scatter_( | |
| 2, | |
| prompt_index.to(torch.int64).view(batch_size, 1, 1).expand(-1, time_steps, -1), | |
| 1.0, | |
| ) |
| ARG QNN_SDK_VERSION=2.40.0.251030 | ||
| ARG QNN_TOOLKIT_URL=https://huggingface.co/csukuangfj/qnn-toolkit/resolve/main/v${QNN_SDK_VERSION}.zip | ||
|
|
There was a problem hiding this comment.
The ANDROID_NDK_VERSION argument is referenced in the LABEL instruction on line 10, but it is not declared until line 46. In Dockerfiles, ARG variables must be declared before they are used, otherwise they evaluate to empty strings. Declaring ARG ANDROID_NDK_VERSION earlier resolves this issue.
ARG QNN_SDK_VERSION=2.40.0.251030
ARG QNN_TOOLKIT_URL=https://huggingface.co/csukuangfj/qnn-toolkit/resolve/main/v${QNN_SDK_VERSION}.zip
ARG ANDROID_NDK_VERSION=29.0.14206865
| subsampling_factor_ = window_shift_ / num_encoder_frames_; | ||
| if (subsampling_factor_ <= 0) { | ||
| SHERPA_ONNX_LOGE( | ||
| "The joiner input '%s' should be [1, %d, 1]. Actual: [%d, %d, %d]", | ||
| joiner_encoder_input_name_.c_str(), encoder_out_dim_, | ||
| encoder_in_shape[0], encoder_in_shape[1], encoder_in_shape[2]); | ||
| "Invalid subsampling factor: %d (window_shift=%d, " | ||
| "num_encoder_frames=%d)", | ||
| subsampling_factor_, window_shift_, num_encoder_frames_); | ||
| SHERPA_ONNX_EXIT(-1); | ||
| } |
There was a problem hiding this comment.
If num_encoder_frames_ is calculated as 0 (e.g., due to an unexpected or malformed model output shape), the division window_shift_ / num_encoder_frames_ will cause a division-by-zero crash. Adding a defensive check for num_encoder_frames_ <= 0 before the division prevents this crash.
if (num_encoder_frames_ <= 0) {
SHERPA_ONNX_LOGE(
"Invalid num_encoder_frames: %d. It must be greater than 0.",
num_encoder_frames_);
SHERPA_ONNX_EXIT(-1);
}
subsampling_factor_ = window_shift_ / num_encoder_frames_;
if (subsampling_factor_ <= 0) {
SHERPA_ONNX_LOGE(
"Invalid subsampling factor: %d (window_shift=%d, "
"num_encoder_frames=%d)",
subsampling_factor_, window_shift_, num_encoder_frames_);
SHERPA_ONNX_EXIT(-1);
}There was a problem hiding this comment.
Pull request overview
This PR adds a QNN export/packaging pipeline for nemotron-3.5-asr-streaming-0.6b and updates the QNN NeMo transducer runtime to support multilingual prompting (prompt_index) and filter language-tag tokens from recognition results.
Changes:
- Extend the QNN NeMo transducer path to pass per-stream language via
prompt_index, and filter<xx>/<xx-YY>language-tag tokens from output. - Update Online/Offline stream option getters to support a fallback default value.
- Add/adjust GitHub Actions workflows and helper scripts for exporting ONNX + producing QNN artifacts; update the QNN SDK Dockerfile metadata.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
sherpa-onnx/csrc/qnn/online-recognizer-nemo-transducer-qnn-impl.h |
Pass language prompt id into QNN encoder; filter language-tag tokens; adjust blank bootstrap behavior. |
sherpa-onnx/csrc/qnn/online-nemo-transducer-model-qnn.h |
Extend QNN encoder API to accept prompt_index; add language→prompt-id helper declaration. |
sherpa-onnx/csrc/qnn/online-nemo-transducer-model-qnn.cc |
Implement prompt_index input plumbing; derive encoder output layout; hardcode prompt dictionary. |
sherpa-onnx/csrc/online-stream.h |
Change option getter to return default when missing (now returns std::string). |
sherpa-onnx/csrc/online-stream.cc |
Implement updated GetOption(key, default) behavior. |
sherpa-onnx/csrc/offline-stream.h |
Change option getter to return default when missing (now returns std::string). |
sherpa-onnx/csrc/offline-stream.cc |
Implement updated GetOption(key, default) behavior. |
scripts/nemo/qnn/nemotron-speech-streaming-en-0.6b/test_onnx.py |
Rename joiner output variable from log_probs to logits. |
scripts/nemo/qnn/nemotron-3.5-asr-streaming-0.6b/wrapper.py |
New exporter wrapper to produce QNN-friendly ONNX for Nemotron 3.5 streaming ASR. |
scripts/nemo/qnn/nemotron-3.5-asr-streaming-0.6b/test_onnx.py |
New ONNX test driver for the exported Nemotron 3.5 streaming model. |
scripts/nemo/qnn/nemotron-3.5-asr-streaming-0.6b/run.sh |
New helper script to install deps and run the export wrapper. |
scripts/nemo/nemotron-3.5-asr-streaming-0.6b/export_onnx.py |
Make export parameterized by chunk size and export one chunk size per run. |
.github/workflows/qnn-sdk-docker-build.yaml |
Update the branch trigger name for the QNN SDK docker build workflow. |
.github/workflows/export-nemotron-3.5-asr-streaming-0.6b.yaml |
Matrix-based chunk-size export; simplify collection/testing per chunk size. |
.github/workflows/export-nemotron-3.5-asr-streaming-0.6b-qnn.yaml |
New workflow to export ONNX artifacts and convert them to QNN binaries/libs per SoC. |
.github/scripts/export-qnn/generate_nemotron_35_asr_streaming.py |
New build-matrix generator for the QNN workflow. |
.github/scripts/export-qnn/Dockerfile-2.40 |
Improve QNN SDK container labels and dependency setup; pin numpy<2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Returns the value for the given key, or default_value if the key | ||
| // does not exist. No exception is thrown for missing keys. | ||
| const std::string &GetOption(const std::string &key) const; | ||
| std::string GetOption(const std::string &key, | ||
| const std::string &default_value = "") const; | ||
|
|
| // Returns the value for the given key, or default_value if the key | ||
| // does not exist. No exception is thrown for missing keys. | ||
| const std::string &GetOption(const std::string &key) const; | ||
| std::string GetOption(const std::string &key, | ||
| const std::string &default_value = "") const; | ||
|
|
| const auto &last = s->GetQnnResult(); | ||
| if (!last.tokens.empty() && last.tokens.back() != blank_id_ && | ||
| static_cast<int32_t>(last.tokens.size()) > 1) { | ||
| s->GetCurrentSegment() += 1; | ||
| } |
| #include <algorithm> | ||
| #include <cmath> | ||
| #include <memory> | ||
| #include <numeric> | ||
| #include <sstream> | ||
| #include <string> | ||
| #include <unordered_set> | ||
| #include <utility> | ||
| #include <vector> |
| OnlineTransducerDecoderResultNoOrt ans; | ||
| ans.frame_offset = src.frame_offset; | ||
| ans.num_trailing_blanks = src.num_trailing_blanks; | ||
| ans.decoder_out = src.decoder_out; | ||
| ans.states = src.states; |
| auto enc_out_shape = encoder_->TensorShape("encoder_out"); | ||
| int64_t total = enc_out_shape[1] * enc_out_shape[2]; | ||
| num_encoder_frames_ = static_cast<int32_t>(total / encoder_out_dim_); | ||
|
|
||
| // If encoder_out is [1, encoder_out_dim, num_frames], we need to | ||
| // transpose it to [1, num_frames, encoder_out_dim] before returning. | ||
| // If it is [1, num_frames, encoder_out_dim], no transpose needed. | ||
| encoder_out_needs_transpose_ = (enc_out_shape[1] == encoder_out_dim_); | ||
|
|
| // Get the prompt ID for a given language (e.g., "en-US", "zh-CN"). | ||
| // Returns 'auto' if the language is not found and logs available languages. | ||
| int32_t GetLanguagePromptId(const std::string &language) const; |
| import json | ||
|
|
||
| from device_info import soc_info_dict | ||
| from dataclasses import asdict, dataclass | ||
| import itertools |
| # See https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-container-registry#labelling-container-images | ||
|
|
||
| LABEL org.opencontainers.image.title="QNN SDK ${QNN_SDK_VERSION}" | ||
| LABEL org.opencontainers.image.description="Pre-built development environment for Qualcomm AI Engine Direct (QNN) SDK ${QNN_SDK_VERSION}, used by sherpa-onnx to convert and run ASR/TTS models on Qualcomm Snapdragon chipsets (e.g. SM8850). Includes: Android NDK r29 (${ANDROID_NDK_VERSION}) for cross-compiling to aarch64-android; Python 3.10 virtualenv with PyTorch, ONNX, onnxruntime, and QNN Python dependencies; QNN CLI tools (qnn-onnx-converter, qnn-model-lib-generator, qnn-context-binary-generator, qnn-net-run) for quantizing ONNX models to QNN context binaries targeting HTP/DSP backends. Typical workflow: mount model files, source envsetup.sh, run qnn-onnx-converter to quantize, then qnn-context-binary-generator to produce .bin files deployable on-device." | ||
| LABEL org.opencontainers.image.source="https://github.com/k2-fsa/sherpa-onnx" | ||
| LABEL org.opencontainers.image.licenses="Apache-2.0" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
.github/scripts/export-qnn/generate_nemotron_35_asr_streaming.py (1)
27-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider documenting why SM8350 is skipped.
The
continueforSM8350is undocumented. A brief comment would help future maintainers understand whether this is a permanent exclusion (e.g., unsupported by this model) or temporary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/export-qnn/generate_nemotron_35_asr_streaming.py around lines 27 - 29, The SM8350 branch in the loop over soc_info_dict is an undocumented exclusion, so add a brief comment near the existing continue in generate_nemotron_35_asr_streaming.py explaining whether SM8350 is intentionally unsupported or only skipped for now. Keep the note close to the model check so future maintainers can understand the intent when reviewing the soc.model.name comparison..github/workflows/export-nemotron-3.5-asr-streaming-0.6b-qnn.yaml (1)
1-420: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd an explicit
permissionsblock to follow least-privilege and ensure release steps have required scopes.The workflow has no top-level or job-level
permissionsblock, so all jobs use the repository default token permissions. The release steps on lines 403–419 that omitrepo_tokenrely onGITHUB_TOKENand needcontents: writeto upload release assets. Without an explicit grant, these steps will fail if the repository defaults to read-only. Adding a top-levelpermissions: contents: readand job-specific overrides also addresses the excessive-permissions concern.Proposed additions
permissions: contents: read jobs: onnx: permissions: actions: write # generate_build_matrix inherits top-level (contents: read) qnn: permissions: contents: write actions: read🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/export-nemotron-3.5-asr-streaming-0.6b-qnn.yaml around lines 1 - 420, The workflow is missing an explicit permissions policy, so the default GITHUB_TOKEN scope may be too broad and the release upload steps in the qnn job can fail without contents: write. Add a top-level permissions block to set least-privilege defaults, then override permissions at the job level for qnn so the Release steps can write release assets while the other jobs stay read-only. Use the existing qnn, onnx, and generate_build_matrix job definitions to place the permissions in the right scope.Source: Linters/SAST tools
.github/workflows/export-nemotron-3.5-asr-streaming-0.6b.yaml (1)
52-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest step lacks output validation for recognition results.
The test step runs
sherpa-onnxbut does not assert or grep the output, so it only verifies the models load without crashing—not that recognition produces correct text. Consider adding a minimal assertion (e.g., checking for expected keywords in stdout) to catch silent regressions in exported models.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/export-nemotron-3.5-asr-streaming-0.6b.yaml around lines 52 - 95, The `Test ${{ matrix.chunk_size_ms }}ms onnx models` step only checks that `sherpa-onnx` runs, not that it returns a valid transcription. Update this workflow step to capture the CLI output from the `sherpa-onnx` invocations and add a minimal assertion against stdout (for example, grep for an expected keyword or non-empty recognized text) so the `chunk`/`src` model checks in the test step verify recognition behavior, not just process startup.scripts/nemo/qnn/nemotron-3.5-asr-streaming-0.6b/test_onnx.py (1)
303-343: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
if True:with a debug flag.The
if True:block unconditionally dumps encoder/decoder/joiner.rawdebug files and.txtmanifests to disk. This is clearly a debug artifact that should be gated behind a--debugcommand-line argument to avoid cluttering the working directory during normal test runs.♻️ Proposed fix
def get_args(): parser = argparse.ArgumentParser( formatter_class=argparse.ArgumentDefaultsHelpFormatter ) parser.add_argument( "--wav", type=str, required=True, ) + parser.add_argument( + "--debug", + action="store_true", + help="Dump intermediate tensors to .raw files", + ) return parser.parse_args()- if True: + if args.debug: with open(f"{name}-encoder.txt", "w") as f:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/nemo/qnn/nemotron-3.5-asr-streaming-0.6b/test_onnx.py` around lines 303 - 343, The unconditional `if True:` block in `test_onnx.py` is a debug artifact that always writes encoder/decoder/joiner `.raw` files and manifest `.txt` files. Replace it with a proper debug gate, ideally driven by an existing or new `--debug` CLI flag in the `main`/argument-parsing path, and wrap the file-dumping logic in that condition so normal test runs do not clutter the working directory. Use the existing `encoder_input_list`, `decoder_input_list`, and `joiner_input_list` dumping section as the guarded block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/export-nemotron-3.5-asr-streaming-0.6b-qnn.yaml:
- Line 75: The fallback after the `ls -lh *.raw` command is incorrect because
`head` may read from stdin and is not a safe no-op in this workflow. Update the
affected steps in the GitHub Actions workflow to use a harmless fallback such as
`|| true` instead of `|| head`, so the logging in this shell snippet remains
explicit and robust when no `.raw` files exist.
- Line 223: The workflow is referencing matrix.model_type even though the
generated build matrix from Config in generate_nemotron_35_asr_streaming.py does
not define that field, so the step name and artifact name resolve incorrectly.
Either add a meaningful model_type property to the Config dataclass and ensure
it is populated in the matrix generation, or remove matrix.model_type from the
“Run ${{ matrix.model_type }}” step and the artifact naming logic so both use
only defined matrix symbols.
In @.github/workflows/export-nemotron-3.5-asr-streaming-0.6b.yaml:
- Line 24: The checkout step currently relies on the default credential
persistence, which is unnecessary for this workflow. Update the
actions/checkout@v4 step to disable persisted GitHub credentials by setting
persist-credentials to false, since the workflow only reads the repo and does
not need to reuse the token later.
In `@sherpa-onnx/csrc/qnn/online-nemo-transducer-model-qnn.cc`:
- Around line 739-764: In online-nemo-transducer-model-qnn.cc, the shape-derived
calculations around encoder output handling in the encoder init logic are
missing validation for `encoder_out_dim_`, which can make the `total /
encoder_out_dim_` and `window_shift_ / num_encoder_frames_` divisions unsafe.
Add an early check after reading `encoder_in_shape[1]` in the initialization
path that verifies the dimension is positive before computing
`num_encoder_frames_`, `encoder_out_needs_transpose_`, and
`subsampling_factor_`, and fail fast with the existing error/exit pattern if the
shape is invalid.
---
Nitpick comments:
In @.github/scripts/export-qnn/generate_nemotron_35_asr_streaming.py:
- Around line 27-29: The SM8350 branch in the loop over soc_info_dict is an
undocumented exclusion, so add a brief comment near the existing continue in
generate_nemotron_35_asr_streaming.py explaining whether SM8350 is intentionally
unsupported or only skipped for now. Keep the note close to the model check so
future maintainers can understand the intent when reviewing the soc.model.name
comparison.
In @.github/workflows/export-nemotron-3.5-asr-streaming-0.6b-qnn.yaml:
- Around line 1-420: The workflow is missing an explicit permissions policy, so
the default GITHUB_TOKEN scope may be too broad and the release upload steps in
the qnn job can fail without contents: write. Add a top-level permissions block
to set least-privilege defaults, then override permissions at the job level for
qnn so the Release steps can write release assets while the other jobs stay
read-only. Use the existing qnn, onnx, and generate_build_matrix job definitions
to place the permissions in the right scope.
In @.github/workflows/export-nemotron-3.5-asr-streaming-0.6b.yaml:
- Around line 52-95: The `Test ${{ matrix.chunk_size_ms }}ms onnx models` step
only checks that `sherpa-onnx` runs, not that it returns a valid transcription.
Update this workflow step to capture the CLI output from the `sherpa-onnx`
invocations and add a minimal assertion against stdout (for example, grep for an
expected keyword or non-empty recognized text) so the `chunk`/`src` model checks
in the test step verify recognition behavior, not just process startup.
In `@scripts/nemo/qnn/nemotron-3.5-asr-streaming-0.6b/test_onnx.py`:
- Around line 303-343: The unconditional `if True:` block in `test_onnx.py` is a
debug artifact that always writes encoder/decoder/joiner `.raw` files and
manifest `.txt` files. Replace it with a proper debug gate, ideally driven by an
existing or new `--debug` CLI flag in the `main`/argument-parsing path, and wrap
the file-dumping logic in that condition so normal test runs do not clutter the
working directory. Use the existing `encoder_input_list`, `decoder_input_list`,
and `joiner_input_list` dumping section as the guarded block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf35c0e1-ee4e-4c21-ac5b-c5a98b218b65
📒 Files selected for processing (17)
.github/scripts/export-qnn/Dockerfile-2.40.github/scripts/export-qnn/generate_nemotron_35_asr_streaming.py.github/workflows/export-nemotron-3.5-asr-streaming-0.6b-qnn.yaml.github/workflows/export-nemotron-3.5-asr-streaming-0.6b.yaml.github/workflows/qnn-sdk-docker-build.yamlscripts/nemo/nemotron-3.5-asr-streaming-0.6b/export_onnx.pyscripts/nemo/qnn/nemotron-3.5-asr-streaming-0.6b/run.shscripts/nemo/qnn/nemotron-3.5-asr-streaming-0.6b/test_onnx.pyscripts/nemo/qnn/nemotron-3.5-asr-streaming-0.6b/wrapper.pyscripts/nemo/qnn/nemotron-speech-streaming-en-0.6b/test_onnx.pysherpa-onnx/csrc/offline-stream.ccsherpa-onnx/csrc/offline-stream.hsherpa-onnx/csrc/online-stream.ccsherpa-onnx/csrc/online-stream.hsherpa-onnx/csrc/qnn/online-nemo-transducer-model-qnn.ccsherpa-onnx/csrc/qnn/online-nemo-transducer-model-qnn.hsherpa-onnx/csrc/qnn/online-recognizer-nemo-transducer-qnn-impl.h
|
|
||
| ls -lh *.onnx* | ||
| ls -lh *.txt | ||
| ls -lh *.raw || head |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
|| head is not a meaningful fallback for ls and may hang on stdin.
When no .raw files exist, ls fails and head is executed without arguments, reading from stdin. In GitHub Actions stdin is typically empty so this returns immediately, but the pattern is fragile and confusing. Use || true instead.
Proposed fix
- ls -lh *.raw || head
+ ls -lh *.raw || trueAlso applies to: 252-252
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/export-nemotron-3.5-asr-streaming-0.6b-qnn.yaml at line
75, The fallback after the `ls -lh *.raw` command is incorrect because `head`
may read from stdin and is not a safe no-op in this workflow. Update the
affected steps in the GitHub Actions workflow to use a harmless fallback such as
`|| true` instead of `|| head`, so the logging in this shell snippet remains
explicit and robust when no `.raw` files exist.
| run: | | ||
| df -h | ||
|
|
||
| - name: Run ${{ matrix.model_type }} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
matrix.model_type is not defined in the generated build matrix.
The Config dataclass in generate_nemotron_35_asr_streaming.py (lines 11–17) defines soc, soc_id, arch, chunk_size_ms, and model_name — but not model_type. As a result, ${{ matrix.model_type }} expands to an empty string, producing a step name of "Run " on line 223 and an artifact name with a double dash (e.g., nemotron-3.5-asr-streaming-0.6b-SM8850-80--json) on line 378.
Either add a model_type field to the Config dataclass if it carries meaningful information, or remove the reference from both lines.
Also applies to: 378-378
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/export-nemotron-3.5-asr-streaming-0.6b-qnn.yaml at line
223, The workflow is referencing matrix.model_type even though the generated
build matrix from Config in generate_nemotron_35_asr_streaming.py does not
define that field, so the step name and artifact name resolve incorrectly.
Either add a meaningful model_type property to the Config dataclass and ensure
it is populated in the matrix generation, or remove matrix.model_type from the
“Run ${{ matrix.model_type }}” step and the artifact naming logic so both use
only defined matrix symbols.
| chunk_size_ms: [80, 160, 320, 560, 1120] | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Set persist-credentials: false on checkout to avoid credential persistence.
The actions/checkout@v4 action persists the GitHub token in .git/config by default. This workflow does not push back to the origin repository (HuggingFace publish clones a separate repo; release upload uses its own token), so persisting credentials is unnecessary and increases the attack surface if any step inadvertently exposes the .git directory.
🔒 Proposed fix
- uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 24-24: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/export-nemotron-3.5-asr-streaming-0.6b.yaml at line 24,
The checkout step currently relies on the default credential persistence, which
is unnecessary for this workflow. Update the actions/checkout@v4 step to disable
persisted GitHub credentials by setting persist-credentials to false, since the
workflow only reads the repo and does not need to reuse the token later.
Source: Linters/SAST tools
| encoder_out_dim_ = encoder_in_shape[1]; | ||
|
|
||
| // Compute num_encoder_frames from the encoder output shape and | ||
| // encoder_out_dim (read from the joiner input above). | ||
| // encoder_out shape differs between models: | ||
| // nemotron-speech-streaming-en-0.6b: [1, num_frames, encoder_out_dim] | ||
| // nemotron-3.5-asr-streaming-0.6b: [1, encoder_out_dim, num_frames] | ||
| // We use encoder_out_dim to determine which dimension is num_frames | ||
| // and whether the encoder output needs to be transposed. | ||
| auto enc_out_shape = encoder_->TensorShape("encoder_out"); | ||
| int64_t total = enc_out_shape[1] * enc_out_shape[2]; | ||
| num_encoder_frames_ = static_cast<int32_t>(total / encoder_out_dim_); | ||
|
|
||
| // If encoder_out is [1, encoder_out_dim, num_frames], we need to | ||
| // transpose it to [1, num_frames, encoder_out_dim] before returning. | ||
| // If it is [1, num_frames, encoder_out_dim], no transpose needed. | ||
| encoder_out_needs_transpose_ = (enc_out_shape[1] == encoder_out_dim_); | ||
|
|
||
| subsampling_factor_ = window_shift_ / num_encoder_frames_; | ||
| if (subsampling_factor_ <= 0) { | ||
| SHERPA_ONNX_LOGE( | ||
| "The joiner input '%s' should be [1, %d, 1]. Actual: [%d, %d, %d]", | ||
| joiner_encoder_input_name_.c_str(), encoder_out_dim_, | ||
| encoder_in_shape[0], encoder_in_shape[1], encoder_in_shape[2]); | ||
| "Invalid subsampling factor: %d (window_shift=%d, " | ||
| "num_encoder_frames=%d)", | ||
| subsampling_factor_, window_shift_, num_encoder_frames_); | ||
| SHERPA_ONNX_EXIT(-1); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="sherpa-onnx/csrc/qnn/online-nemo-transducer-model-qnn.cc"
# Show the surrounding function with line numbers.
sed -n '700,790p' "$file" | cat -n
# Search for related validation and tensor-shape handling in this file.
rg -n "encoder_out_dim_|num_encoder_frames_|window_shift_|TensorShape\\(\"encoder_out\"\\)|Invalid subsampling factor" "$file"Repository: k2-fsa/sherpa-onnx
Length of output: 5874
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="sherpa-onnx/csrc/qnn/online-nemo-transducer-model-qnn.cc"
# Inspect the encoder shape handling and earlier validation around window_shift_.
sed -n '540,590p' "$file" | cat -n
# Inspect the code that consumes encoder_out_dim_ and num_encoder_frames_.
sed -n '780,810p' "$file" | cat -n
# Search for any explicit guards on encoder_out_dim_ or num_encoder_frames_.
rg -n "encoder_out_dim_ <=|num_encoder_frames_ <=|encoder_out_dim_|num_encoder_frames_" "$file"Repository: k2-fsa/sherpa-onnx
Length of output: 4883
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the TensorShape implementation and any documentation about returned dims.
rg -n "TensorShape\\(" -g '!**/build/**' -g '!**/dist/**' sherpa-onnx k2-fsa 2>/dev/null || true
# Try to locate the model wrapper that defines TensorShape.
fd -a ".*(model|tensor|qnn).*\\.(cc|h|hpp|cxx|hh)$" sherpa-onnx | head -n 50
# Search for handling of unknown / dynamic dimensions.
rg -n "dynamic|unknown|dim.*-1|shape.*-1|TensorShape.*int64_t|std::vector<int64_t>" sherpa-onnx/csrc 2>/dev/null || trueRepository: k2-fsa/sherpa-onnx
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the QNN shape API implementation and how shapes are represented.
sed -n '250,340p' sherpa-onnx/csrc/qnn/qnn-model.cc | cat -n
sed -n '1,120p' sherpa-onnx/csrc/qnn/qnn-model.h | cat -n
# Inspect the exact validation block in the Nemo QNN model.
sed -n '724,770p' sherpa-onnx/csrc/qnn/online-nemo-transducer-model-qnn.cc | cat -n
# Look for any explicit checks against zero / negative dims in nearby QNN model code.
rg -n "<= 0|< 0|dynamic|unknown|replace dynamic dims|TensorShape\\(\"encoder_out\"\\)|GetShape\\(" sherpa-onnx/csrc/qnn/*.cc sherpa-onnx/csrc/qnn/*.hRepository: k2-fsa/sherpa-onnx
Length of output: 11108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect a similar QNN transducer model that already validates these derived sizes.
sed -n '260,340p' sherpa-onnx/csrc/qnn/offline-zipformer-transducer-model-qnn.cc | cat -n
# Inspect the same kind of logic in another QNN model for comparison.
sed -n '300,350p' sherpa-onnx/csrc/qnn/offline-parakeet-tdt-model-qnn.cc | cat -nRepository: k2-fsa/sherpa-onnx
Length of output: 5883
Guard the derived shape divisions. encoder_in_shape[1] is still unchecked, so a zero/negative encoder_out_dim_ can make total / encoder_out_dim_ and then window_shift_ / num_encoder_frames_ unsafe. Add a positive-dimension check before those divisions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sherpa-onnx/csrc/qnn/online-nemo-transducer-model-qnn.cc` around lines 739 -
764, In online-nemo-transducer-model-qnn.cc, the shape-derived calculations
around encoder output handling in the encoder init logic are missing validation
for `encoder_out_dim_`, which can make the `total / encoder_out_dim_` and
`window_shift_ / num_encoder_frames_` divisions unsafe. Add an early check after
reading `encoder_in_shape[1]` in the initialization path that verifies the
dimension is positive before computing `num_encoder_frames_`,
`encoder_out_needs_transpose_`, and `subsampling_factor_`, and fail fast with
the existing error/exit pattern if the shape is invalid.
8c83c46 to
3c02b87
Compare
|
Hi Fangjun, thanks so much for the tag, and for all the Nemotron streaming work. We build Anti-Vocale, an on-device Italian voice-message transcriber for Android (sherpa-onnx under the hood); the multilingual Nemotron path is one of our streaming backends, and the per-stream language conditioning from #3671 was the real unlock for our multilingual users. The QNN path is exciting to see. One thing that may be useful for planning: our primary test device is a Realme GT 6T, i.e. SM7675 / Snapdragon 7+ Gen 3, so the SM8850 binary won't run on it directly. We'd be very keen to try QNN once 7-Gen-3-class SoCs are in the build matrix, and happy to test an SM7675 build if that's ever on the roadmap. Separately, thanks for the Thanks again, really appreciated. |
|
Glad to see NPU work landing on the Nemotron streaming path. On-device acceleration for the multilingual model on Android is something we'd love to use in Hedy. The blocker for us is the per-SoC prebuilt context binary model. We ship to a really fragmented Android base: a long tail of Qualcomm SoCs, plus a big chunk of non-Qualcomm devices (MediaTek, Exynos, Tensor). Shipping and maintaining a separate prebuilt binary per SoC just isn't realistic for us, and since QNN wouldn't help the non-Qualcomm devices at all, we'd still be carrying the CPU ONNX path as the universal fallback anyway. So for us QNN ends up as a bonus for a slice of flagship Qualcomm users rather than something we could actually standardize on. The thing that would flip that for us is a runtime-prepared path: one portable artifact that does the HTP graph prepare on-device at first run, instead of prebuilt per-SoC binaries. A one-time compile on first launch is fine. A per-SoC distribution matrix isn't. Sharing this mainly in case it's useful for prioritization. For a broadly distributed consumer app, on-device prepare is what would make NPU acceleration realistic to adopt. For now we're staying on CPU ONNX. Thanks for all the Nemotron streaming work, the per-stream language conditioning has been a real unlock for our multilingual users. |
See #3671 #3664
cc @paoloantinori @JulianPscheid
See models at
In the following, we show how to run it on Xiaomi 17 Pro, which has Qualcomm SM8850
Usage
Download a model (with context binary files)
Run it
Please follow https://k2-fsa.github.io/sherpa/onnx/qnn/run-executables-on-your-phone-binary.html
The logs are:
Summary by CodeRabbit
New Features
Bug Fixes