Export Paraformer ASR models to QNN - #2925
Conversation
dosubot
Bot
added
the
size:XXL
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a complete QNN export pipeline for Paraformer: CI workflow to drive multi-SOC/framework exports, conversion scripts (encoder/predictor/decoder), data generators and tests, RKNN export tweaks (opset/forward monkeypatch), and packaging/release steps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Shell as Conversion Script
participant ONNX as ONNX Exporter
participant Data as Test Data Generator
participant QConv as qnn-onnx-converter
participant QLib as qnn-model-lib-generator
participant QBin as qnn-context-binary-generator
participant Storage as Artifact Packager
GH->>Shell: trigger job (matrix: soc, framework, t)
Shell->>ONNX: run export_*_onnx.py -> produce .onnx
Shell->>Data: run generate_*_data.py -> produce .raw + -list.txt
Shell->>QConv: qnn-onnx-converter (use -list.txt, dtypes)
QConv-->>Shell: quantized artifact (.cpp/.qnn)
Shell->>QLib: generate_config.py / qnn-model-lib-generator
QLib-->>Shell: model libs (.so / .a)
Shell->>QBin: qnn-context-binary-generator (backend, config)
QBin-->>Shell: final binary/context
Shell->>Storage: tar.bz2 packaging
Storage-->>GH: upload artifact (+ optional release)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Summary of ChangesHello @csukuangfj, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes the foundational framework for exporting Paraformer Automatic Speech Recognition (ASR) models to the Qualcomm Neural Processing SDK (QNN). By providing a suite of new scripts and modifying existing ONNX export utilities, it streamlines the process of converting PyTorch-based ASR models into a format optimized for Qualcomm hardware. This enhancement is crucial for enabling efficient, on-device inference of ASR capabilities, with the C++ runtime integration planned for a subsequent pull request. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 adds scripts to export Paraformer ASR models to the QNN format. The changes include new shell scripts for the conversion workflow, Python scripts for data generation, and modifications to existing ONNX export scripts. My review has identified a few critical issues that will prevent the new scripts from running correctly. The most significant is that several Python files intended for code reuse appear to have been added as text files instead of symbolic links, which will cause execution and import errors. There is also a syntax error in one of the test scripts. Additionally, I've provided suggestions to improve the robustness of the new shell scripts by quoting variables and adding error handling, and to improve code maintainability by removing duplicated code. Addressing these points will ensure the new workflow is functional and easier to maintain.
| @@ -0,0 +1 @@ | |||
| ../rknn/export_decoder_onnx.py No newline at end of file | |||
There was a problem hiding this comment.
This file, and others like it in this directory (export_encoder_onnx.py, export_predictor_onnx.py, test_onnx.py, torch_model.py), appear to be intended as symbolic links but have been committed as text files. This will cause both script execution (e.g., python3 ./export_decoder_onnx.py) and Python imports (from export_decoder_onnx import ...) to fail.
The recommended fix is to replace these text files with actual symbolic links. You can do this with a command like:
rm scripts/paraformer/qnn/export_decoder_onnx.py
ln -s ../rknn/export_decoder_onnx.py scripts/paraformer/qnn/export_decoder_onnx.pyThis should be done for all 5 files that are reusing code from the rknn directory.
| print("predictor params (M)", predictor_params / 1024 / 1024) | ||
| print("decoder params (M)", decoder_params / 1024 / 1024) | ||
|
|
||
| features = np.load |
There was a problem hiding this comment.
This line features = np.load is incomplete and will cause a TypeError at runtime. It appears to be a leftover from development and should be removed, as the features variable is correctly assigned on the next line.
| if [ -z $t ]; then | ||
| echo "Please run export t=num_input_seconds" | ||
| exit -1 | ||
| fi |
There was a problem hiding this comment.
Shell variables should be quoted (e.g., "$t") to prevent word splitting and globbing issues, which can cause bugs if the variable is empty or contains spaces. Also, exit -1 is non-standard; exit 1 is the convention for signaling an error. This applies to all variable checks in this script and the other convert_*.sh scripts.
| if [ -z $t ]; then | |
| echo "Please run export t=num_input_seconds" | |
| exit -1 | |
| fi | |
| if [ -z "$t" ]; then | |
| echo "Please run export t=num_input_seconds" | |
| exit 1 | |
| fi |
| @@ -0,0 +1,47 @@ | |||
| #!/usr/bin/env python3 | |||
| # Copyright 2025 Xiaomi Corp. (authors: Fangjun Kuang) | |||
There was a problem hiding this comment.
The copyright year is set to 2025, which is in the future. Please update it to the current year, 2024.
| # Copyright 2025 Xiaomi Corp. (authors: Fangjun Kuang) | |
| # Copyright 2024 Xiaomi Corp. (authors: Fangjun Kuang) |
| @@ -0,0 +1,84 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
For more robust shell scripting, it's a best practice to add set -euo pipefail at the beginning of the script. This will make the script exit on errors, treat unset variables as errors, and handle pipeline failures correctly. This advice also applies to convert_encoder.sh and convert_predictor.sh.
| #!/usr/bin/env bash | |
| #!/usr/bin/env bash | |
| set -euo pipefail |
| def pad(features, max_len): | ||
| if features.shape[0] > max_len: | ||
| return features[:max_len] | ||
| elif features.shape[0] < max_len: | ||
| features = np.pad( | ||
| features, | ||
| ((0, max_len - features.shape[0]), (0, 0)), | ||
| mode="constant", | ||
| constant_values=0, | ||
| ) | ||
| return features |
There was a problem hiding this comment.
The pad function is duplicated in generate_decoder_data.py, generate_encoder_data.py, and generate_predictor_data.py. To adhere to the DRY (Don't Repeat Yourself) principle and improve maintainability, this function should be extracted into a shared utility file (e.g., utils.py) and imported where needed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (3)
scripts/paraformer/qnn/export_decoder_onnx.py (1)
1-1: Same reference pattern issue as other QNN export scripts.See comment on
export_predictor_onnx.pyregarding the non-standard reference file pattern.scripts/paraformer/qnn/test_onnx.py (1)
1-1: Same reference pattern issue as other QNN scripts.See comment on
export_predictor_onnx.pyregarding the non-standard reference file pattern.scripts/paraformer/qnn/export_encoder_onnx.py (1)
1-1: Same reference pattern issue as other QNN export scripts.See comment on
export_predictor_onnx.pyregarding the non-standard reference file pattern.
🧹 Nitpick comments (8)
scripts/sense-voice/rknn/test_nano_torch.py (1)
50-51: Consider moving this change to a separate PR.While the parameter counting logic is useful for model inspection, this change is in the
sense-voicedirectory, whereas the PR objectives specifically focus on exporting Paraformer models to QNN. This appears to be an unrelated improvement that would be better tracked separately for clearer change history..github/scripts/export-qnn/generate_paraformer.py (1)
4-8: Consider reordering imports per PEP 8.Standard library imports (
json,itertools) should precede local imports (device_info), anddataclassesshould be grouped with other standard library imports.🔎 Suggested import order
-import json - -from device_info import soc_info_dict -from dataclasses import asdict, dataclass -import itertools +import itertools +import json +from dataclasses import asdict, dataclass + +from device_info import soc_info_dict.github/workflows/export-paraformer-to-qnn.yaml (2)
206-220: Remove or use thedirvariable.The
dir=$PWDassignment on line 220 is unused. Either remove it or export it if needed externally.🔎 Proposed fix
- dir=$PWD - cd scripts/paraformer/qnn
419-457: Consider consolidating release steps using composite conditions.There are four separate release steps with similar configurations. If maintainability becomes a concern, these could be consolidated using conditional logic or reusable workflows.
scripts/paraformer/qnn/generate_encoder_data.py (1)
13-23: Consider extracting the sharedpadfunction to a common module.This
padfunction is duplicated ingenerate_encoder_data.py,generate_predictor_data.py, and likelygenerate_decoder_data.py. While the duplication is acceptable for now given the script's utility nature, consolidating it could improve maintainability.scripts/paraformer/rknn/export_predictor_onnx.py (1)
28-29: Misplacedif __name__ == "__main__":guard creates potential confusion.There are two
if __name__ == "__main__":blocks (lines 28-29 and 58-60). The monkeypatch at line 29 will execute when the script runs directly, but the structure is unconventional. Themain()function definition at line 32 appears between these two guards.Consider either:
- Moving the monkeypatch inside
main()before it's needed, or- Placing it at module scope (outside the guard) since other files like
generate_decoder_data.pyandtest_qnn.pyimportmodified_predictor_forwardand do their own patching anyway.Option: Move monkeypatch to module scope for consistency
-if __name__ == "__main__": - CifPredictorV2.forward = modified_predictor_forward +CifPredictorV2.forward = modified_predictor_forwardscripts/paraformer/qnn/test_qnn.py (2)
27-32: Minor: Parameter count uses MiB divisor but labels as "M".Dividing by
1024 * 1024gives mebibytes (MiB), while "M" typically implies millions (1e6). This is a common convention in ML, so not blocking, but for precision you could use/ 1e6for actual millions.
77-113: Consider cleaning up or documenting the debug branch.The
if False:block contains an alternative code path for testing with QNN-exported intermediate outputs. While useful during development, consider either:
- Removing it if no longer needed, or
- Converting to a CLI flag or environment variable for controlled testing, or
- Adding a comment explaining its purpose for future maintainers.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/scripts/export-qnn/generate_paraformer.py.github/workflows/export-paraformer-to-qnn.yamlscripts/paraformer/qnn/.gitignorescripts/paraformer/qnn/convert_decoder.shscripts/paraformer/qnn/convert_encoder.shscripts/paraformer/qnn/convert_predictor.shscripts/paraformer/qnn/export_decoder_onnx.pyscripts/paraformer/qnn/export_encoder_onnx.pyscripts/paraformer/qnn/export_predictor_onnx.pyscripts/paraformer/qnn/generate_decoder_data.pyscripts/paraformer/qnn/generate_encoder_data.pyscripts/paraformer/qnn/generate_predictor_data.pyscripts/paraformer/qnn/test_onnx.pyscripts/paraformer/qnn/test_qnn.pyscripts/paraformer/qnn/torch_model.pyscripts/paraformer/rknn/export_decoder_onnx.pyscripts/paraformer/rknn/export_encoder_onnx.pyscripts/paraformer/rknn/export_predictor_onnx.pyscripts/paraformer/rknn/test_onnx.pyscripts/sense-voice/rknn/test_nano_torch.py
🧰 Additional context used
🧬 Code graph analysis (5)
scripts/paraformer/qnn/generate_predictor_data.py (1)
scripts/paraformer/qnn/test_qnn.py (1)
main(25-118)
scripts/paraformer/qnn/generate_decoder_data.py (1)
scripts/paraformer/qnn/test_qnn.py (1)
main(25-118)
scripts/paraformer/rknn/export_predictor_onnx.py (1)
scripts/paraformer/rknn/torch_model.py (1)
CifPredictorV2(1111-1222)
scripts/paraformer/rknn/export_decoder_onnx.py (1)
scripts/paraformer/rknn/export_encoder_onnx.py (3)
load_model(81-111)get_num_input_frames(139-146)get_args(15-34)
scripts/paraformer/qnn/test_qnn.py (1)
scripts/paraformer/rknn/export_predictor_onnx.py (2)
modified_predictor_forward(10-25)main(33-55)
🪛 actionlint (1.7.9)
.github/workflows/export-paraformer-to-qnn.yaml
27-27: shellcheck reported issue in this script: SC2086:info:7:26: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
56-56: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
103-103: shellcheck reported issue in this script: SC2086:info:10:12: Double quote to prevent globbing and word splitting
(shellcheck)
206-206: shellcheck reported issue in this script: SC2034:warning:14:1: dir appears unused. Verify use (or export if used externally)
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:102:8: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:104:4: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:64:7: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:67:8: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:70:4: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:94:9: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:97:10: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2242:error:81:10: Can only exit with status 0-255. Other data should be written to stdout/stderr
(shellcheck)
316-316: shellcheck reported issue in this script: SC2034:warning:1:1: dir appears unused. Verify use (or export if used externally)
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:55:7: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:58:8: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:61:4: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:85:9: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:88:10: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:93:8: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:95:4: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2086:info:24:114: Double quote to prevent globbing and word splitting
(shellcheck)
316-316: shellcheck reported issue in this script: SC2242:error:72:10: Can only exit with status 0-255. Other data should be written to stdout/stderr
(shellcheck)
🪛 Shellcheck (0.11.0)
scripts/paraformer/qnn/convert_predictor.sh
[error] 5-5: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
[error] 10-10: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
[error] 15-15: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
scripts/paraformer/qnn/convert_decoder.sh
[error] 5-5: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
[error] 10-10: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
[error] 15-15: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
scripts/paraformer/qnn/convert_encoder.sh
[error] 5-5: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
[error] 10-10: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
[error] 15-15: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
🔇 Additional comments (21)
scripts/paraformer/qnn/torch_model.py (1)
1-1: No issues found. The file is a proper symlink to a legitimate module.The file
scripts/paraformer/qnn/torch_model.pyis a filesystem symlink pointing to../rknn/torch_model.py, which is a valid approach for sharing the implementation across different quantization backends (QNN and RKNN). The target module contains complete encoder, decoder, and predictor implementations, not a path string.Likely an incorrect or invalid review comment.
scripts/paraformer/rknn/export_decoder_onnx.py (1)
6-9: LGTM! Good addition of configurable arguments.The addition of argument parsing for
input-len-in-seconds,float-mask, andopset-versionprovides the necessary flexibility for RKNN/QNN exports that don't support dynamic shapes. The conditional mask dtype selection is clean and well-structured.Also applies to: 45-60
scripts/paraformer/rknn/test_onnx.py (1)
64-64: LGTM! Cleaner output.Commenting out the debug print statement reduces noise in the output without affecting functionality.
scripts/paraformer/qnn/.gitignore (1)
1-2: LGTM! Appropriate ignore patterns.The patterns correctly exclude generated artifacts (raw feature/binary files and list files) from version control, which aligns with the QNN export workflow described in the PR.
scripts/paraformer/qnn/export_predictor_onnx.py (1)
1-1: This review comment is based on a mischaracterization of the files. The scripts (export_predictor_onnx.py, export_encoder_onnx.py, export_decoder_onnx.py, and test_onnx.py) are complete, valid Python implementations—not stub files or reference documentation. Each file contains proper imports, function definitions, model loading logic, and ONNX export functionality using torch.onnx.export(). They are executable as-is and serve legitimate purposes in the model export workflow. No clarification or refactoring is needed.Likely an incorrect or invalid review comment.
scripts/paraformer/rknn/export_encoder_onnx.py (1)
28-34: LGTM! Good parameterization of the opset version.This change appropriately externalizes the previously hardcoded opset version, enabling the QNN export scripts to use a different opset version (17) while maintaining backwards compatibility with the default value of 14 for RKNN.
.github/scripts/export-qnn/generate_paraformer.py (1)
20-43: LGTM! Clean matrix generation logic.The Cartesian product approach efficiently generates all configuration combinations for the build matrix.
scripts/paraformer/qnn/convert_decoder.sh (1)
18-84: LGTM! Well-structured conversion pipeline.The script follows a clear sequential workflow: ONNX export → data generation → QNN conversion → library building → binary generation. Good use of progress echoes for debugging.
scripts/paraformer/qnn/convert_predictor.sh (1)
18-82: LGTM! Predictor conversion pipeline follows the established pattern.The script mirrors the structure of the encoder and decoder conversion scripts, maintaining consistency across the codebase.
scripts/paraformer/qnn/generate_encoder_data.py (1)
26-53: LGTM! Clean data generation workflow.The script correctly processes WAV files, pads features to the required length, and generates the input list file for QNN conversion.
scripts/paraformer/qnn/convert_encoder.sh (1)
18-81: LGTM! Consistent encoder conversion pipeline.The script follows the same well-structured pattern as the decoder and predictor scripts.
scripts/paraformer/qnn/generate_predictor_data.py (2)
14-15: Module-level monkey-patching noted.The
CifPredictorV2.forward = modified_predictor_forwardassignment at module level is intentional to modify the forward pass behavior for predictor data generation. This is acceptable for the script's purpose but should be documented if the behavior needs to persist across imports.
31-72: LGTM! Predictor data generation with proper tensor layout conversion.The transpose operation (
encoder_out.transpose(1, 2)) correctly converts from (N, T, C) to (N, C, T) layout as required by QNN, and this is well-documented with the inline comment.scripts/paraformer/rknn/export_predictor_onnx.py (2)
10-25: LGTM!The simplified forward pass correctly extracts the alpha computation from the full
CifPredictorV2.forward, matching the convolution, sigmoid, ReLU with smooth_factor/noise_threshold, and squeeze operations. This is appropriate for exporting only the predictor component to ONNX.
44-54: Good improvement: configurable opset version.Using
args.opset_versioninstead of a hardcoded value aligns with the other export scripts and provides flexibility for different ONNX runtime requirements.scripts/paraformer/qnn/generate_decoder_data.py (3)
1-16: LGTM!Clean import structure and appropriate module-level monkeypatch for this standalone data generation script.
18-28: LGTM!The
padfunction correctly handles the three cases (truncate, pad, or pass-through) for fixed-length feature arrays.
73-89: LGTM!The tensor permutation to
(N, C, T)layout is well-documented, and theclone()calls ensure contiguous memory before conversion to numpy. The raw file output pattern is consistent with the test workflow.scripts/paraformer/qnn/test_qnn.py (3)
1-12: LGTM!Consistent import structure and monkeypatch pattern with other QNN scripts.
15-21: LGTM!This implementation reads the actual token ID from the file rather than assuming sequential indices, which is more robust. Note: this differs from
test_onnx.py:load_tokens()which uses incremental indexing.
115-123: LGTM!The token decoding and entry point follow standard patterns consistent with the other scripts.
| for p in x86_64-linux-clang aarch64-android; do | ||
| if [[ $p == x86_64-linux-clang ]]; then | ||
|
|
||
| d=sherpa-onnx-qnn-$t-seconds-paraformer-zh-2023-03-28-int8-linux-x64 | ||
| elif [[ $p == aarch64-android ]]; then | ||
| d=sherpa-onnx-qnn-$t-seconds-paraformer-zh-2023-03-28-int8-android-aarch64 | ||
| else | ||
| echo "Unknown $p" | ||
| exit -1 | ||
| fi |
There was a problem hiding this comment.
Fix invalid exit code in fallback case.
The exit -1 is invalid in Bash. Use exit 1 for error conditions.
🔎 Proposed fix
else
echo "Unknown $p"
- exit -1
+ exit 1
fi🤖 Prompt for AI Agents
.github/workflows/export-paraformer-to-qnn.yaml around lines 279 to 288: the
fallback branch uses an invalid Bash exit status `exit -1`; replace it with a
valid non-zero exit code such as `exit 1` to signal an error. Update the line to
call `exit 1` (or another positive integer) and ensure any surrounding scripts
expecting specific codes are adjusted if needed.
| for p in x86_64-linux-clang aarch64-android; do | ||
| if [[ $p == x86_64-linux-clang ]]; then | ||
|
|
||
| d=sherpa-onnx-qnn-$t-seconds-paraformer-zh-2025-10-07-int8-linux-x64 | ||
| elif [[ $p == aarch64-android ]]; then | ||
| d=sherpa-onnx-qnn-$t-seconds-paraformer-zh-2025-10-07-int8-android-aarch64 | ||
| else | ||
| echo "Unknown $p" | ||
| exit -1 | ||
| fi |
There was a problem hiding this comment.
Fix invalid exit code in WSChuan-ASR fallback case.
Same issue as above—use exit 1 instead of exit -1.
🔎 Proposed fix
else
echo "Unknown $p"
- exit -1
+ exit 1
fi📝 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.
| for p in x86_64-linux-clang aarch64-android; do | |
| if [[ $p == x86_64-linux-clang ]]; then | |
| d=sherpa-onnx-qnn-$t-seconds-paraformer-zh-2025-10-07-int8-linux-x64 | |
| elif [[ $p == aarch64-android ]]; then | |
| d=sherpa-onnx-qnn-$t-seconds-paraformer-zh-2025-10-07-int8-android-aarch64 | |
| else | |
| echo "Unknown $p" | |
| exit -1 | |
| fi | |
| for p in x86_64-linux-clang aarch64-android; do | |
| if [[ $p == x86_64-linux-clang ]]; then | |
| d=sherpa-onnx-qnn-$t-seconds-paraformer-zh-2025-10-07-int8-linux-x64 | |
| elif [[ $p == aarch64-android ]]; then | |
| d=sherpa-onnx-qnn-$t-seconds-paraformer-zh-2025-10-07-int8-android-aarch64 | |
| else | |
| echo "Unknown $p" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In .github/workflows/export-paraformer-to-qnn.yaml around lines 380 to 389, the
fallback branch uses an invalid exit code `exit -1`; replace it with `exit 1` so
the script returns a valid non-zero status on error, updating that single line
to `exit 1`.
| mask = torch.zeros(1, encoder_out.shape[1], dtype=torch.int32) | ||
|
|
||
| mask[0, :num_tokens] = 1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check mask dtype usage across the QNN scripts
rg -n "mask.*dtype" scripts/paraformer/qnn/
rg -n "torch\.(int32|float32)" scripts/paraformer/qnn/Repository: k2-fsa/sherpa-onnx
Length of output: 841
🏁 Script executed:
#!/bin/bash
# Read the specific files mentioned to understand context
echo "=== generate_decoder_data.py lines 60-80 ==="
sed -n '60,80p' scripts/paraformer/qnn/generate_decoder_data.py
echo -e "\n=== test_qnn.py around line 58 ==="
sed -n '50,70p' scripts/paraformer/qnn/test_qnn.pyRepository: k2-fsa/sherpa-onnx
Length of output: 1619
🏁 Script executed:
#!/bin/bash
# Check full context of mask usage in generate_decoder_data.py around lines 69-95
sed -n '65,95p' scripts/paraformer/qnn/generate_decoder_data.pyRepository: k2-fsa/sherpa-onnx
Length of output: 1095
Mask dtype should be torch.float32 to match inference pattern in test_qnn.py.
The mask is created with dtype=torch.int32 in generate_decoder_data.py (line 69), but test_qnn.py creates it with dtype=torch.float32 (line 58) for model inference, only converting to int32 before saving to file (line 68). Since generate_decoder_data.py is preparing data files for inference, the mask should be torch.float32 to maintain consistency with the decoder's inference expectations.
🤖 Prompt for AI Agents
In scripts/paraformer/qnn/generate_decoder_data.py around lines 69 to 71, the
mask is created with dtype=torch.int32 but should be torch.float32 to match the
inference pattern used in test_qnn.py; change the mask creation to use
dtype=torch.float32 (i.e., torch.zeros(..., dtype=torch.float32)) and keep
assigning 1.0 for the valid positions, and if the downstream saving expects
int32 convert only at save time (or keep float if saving for inference) so the
runtime inference mask type matches test_qnn.py.
| def get_args(): | ||
| parser = argparse.ArgumentParser( | ||
| formatter_class=argparse.ArgumentDefaultsHelpFormatter | ||
| ) | ||
|
|
||
| parser.add_argument( | ||
| "--input-len-in-seconds", | ||
| type=int, | ||
| required=True, | ||
| help="""RKNN/QNN does not support dynamic shape, so we need to hard-code | ||
| how long the model can process. | ||
| """, | ||
| ) | ||
|
|
||
| parser.add_argument( | ||
| "--float-mask", | ||
| type=int, | ||
| default=1, | ||
| help="1 to use float master. 0 to use int32 mask", | ||
| ) | ||
|
|
||
| parser.add_argument( | ||
| "--opset-version", | ||
| type=int, | ||
| default=14, | ||
| ) | ||
| return parser.parse_args() |
There was a problem hiding this comment.
Fix typo in help text.
The help text on line 29 contains a typo: "1 to use float master" should be "1 to use float mask".
🔎 Proposed fix
parser.add_argument(
"--float-mask",
type=int,
default=1,
- help="1 to use float master. 0 to use int32 mask",
+ help="1 to use float mask. 0 to use int32 mask",
)🤖 Prompt for AI Agents
In scripts/paraformer/rknn/export_decoder_onnx.py around lines 11 to 37, the
help string for the --float-mask argument contains a typo ("1 to use float
master"); update that help text to read "1 to use float mask" (keep the rest of
the message and formatting intact) so the option description correctly refers to
a "mask" instead of "master".
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
scripts/paraformer/qnn/test_qnn.py (1)
34-34: Remove incomplete statement.Line 34 contains an incomplete assignment that is immediately overwritten on line 35.
.github/workflows/export-paraformer-to-qnn.yaml (2)
287-287: Fix invalid exit code.Line 287 uses
exit -1which is invalid in Bash (only 0-255 are valid). Useexit 1instead.
388-388: Fix invalid exit code.Same issue as line 287—use
exit 1instead ofexit -1.
🧹 Nitpick comments (5)
scripts/paraformer/qnn/test_qnn.py (1)
76-117: Remove or make configurable the unreachable code block.The
if False:block contains an alternative decoder path using precomputed QNN outputs (qnn_encoder_out,qnn_alpha) that will never execute. Additionally, theelsebranch (lines 105-112) duplicates the decoding logic from lines 69-74.Consider either:
- Removing this dead code if it's no longer needed
- Converting
Falseto a command-line flag or environment variable if this is intended for future debuggingscripts/paraformer/qnn/convert_decoder.sh (1)
3-16: Quote variables in test conditions.The variable checks on lines 3, 8, and 13 should quote the variables to prevent word splitting and globbing issues:
[ -z "$t" ],[ -z "$soc" ],[ -z "$QNN_SDK_ROOT" ].🔎 Proposed fix
-if [ -z $t ]; then +if [ -z "$t" ]; then echo "Please run export t=num_input_seconds" exit 1 fi -if [ -z $soc ]; then +if [ -z "$soc" ]; then echo "Please run export soc=SM8850, etc." exit 1 fi -if [ -z $QNN_SDK_ROOT ]; then +if [ -z "$QNN_SDK_ROOT" ]; then echo "Please run setup QNN first" exit 1 fiscripts/paraformer/qnn/convert_predictor.sh (1)
3-16: Quote variables in test conditions.Same as in
convert_decoder.sh, the variable checks should quote the variables:[ -z "$t" ],[ -z "$soc" ],[ -z "$QNN_SDK_ROOT" ].🔎 Proposed fix
-if [ -z $t ]; then +if [ -z "$t" ]; then echo "Please run export t=num_input_seconds" exit 1 fi -if [ -z $soc ]; then +if [ -z "$soc" ]; then echo "Please run export soc=SM8850, etc." exit 1 fi -if [ -z $QNN_SDK_ROOT ]; then +if [ -z "$QNN_SDK_ROOT" ]; then echo "Please run setup QNN first" exit 1 fi.github/workflows/export-paraformer-to-qnn.yaml (2)
220-220: Remove unused variable.The
dirvariable is assigned but never used in the script. Consider removing it.
317-317: Remove unused variable.Same as line 220—the
dirvariable is assigned but never used.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/export-paraformer-to-qnn.yamlscripts/paraformer/qnn/convert_decoder.shscripts/paraformer/qnn/convert_encoder.shscripts/paraformer/qnn/convert_predictor.shscripts/paraformer/qnn/test_qnn.py
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/paraformer/qnn/convert_encoder.sh
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/paraformer/qnn/test_qnn.py (1)
scripts/paraformer/rknn/export_predictor_onnx.py (2)
modified_predictor_forward(10-25)main(33-55)
🪛 actionlint (1.7.9)
.github/workflows/export-paraformer-to-qnn.yaml
27-27: shellcheck reported issue in this script: SC2086:info:7:26: Double quote to prevent globbing and word splitting
(shellcheck)
27-27: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
56-56: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
103-103: shellcheck reported issue in this script: SC2086:info:10:12: Double quote to prevent globbing and word splitting
(shellcheck)
206-206: shellcheck reported issue in this script: SC2034:warning:14:1: dir appears unused. Verify use (or export if used externally)
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:102:8: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:104:4: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:64:7: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:67:8: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:70:4: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:94:9: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2035:info:97:10: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
206-206: shellcheck reported issue in this script: SC2242:error:81:10: Can only exit with status 0-255. Other data should be written to stdout/stderr
(shellcheck)
316-316: shellcheck reported issue in this script: SC2034:warning:1:1: dir appears unused. Verify use (or export if used externally)
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:55:7: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:58:8: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:61:4: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:85:9: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:88:10: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:93:8: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2035:info:95:4: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
316-316: shellcheck reported issue in this script: SC2086:info:24:114: Double quote to prevent globbing and word splitting
(shellcheck)
316-316: shellcheck reported issue in this script: SC2242:error:72:10: Can only exit with status 0-255. Other data should be written to stdout/stderr
(shellcheck)
1 participant
You can download them from
Will add C++ runtime in a separate pull request.
Summary by CodeRabbit
New Features
Tests
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.