Export zipformer ctc models to QNN - #2815
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
dosubot
Bot
added
the
size:L
WalkthroughA GitHub Actions workflow is added that triggers on pushes to the qnn-zipformer-ctc-models branch and manual dispatch. The workflow converts Zipformer CTC models to QNN format using a multi-step pipeline involving environment setup, dependency installation, model conversion, quantization, artifact collection, and conditional release to GitHub Releases. Changes
Sequence DiagramsequenceDiagram
participant Trigger as Trigger (push/dispatch)
participant Setup as Environment Setup
participant Build as Model Conversion
participant Artifact as Artifact Collection
participant Release as Conditional Release
Trigger->>Setup: Workflow triggered
Setup->>Setup: Checkout code
Setup->>Setup: Setup Python & NDK
Setup->>Setup: Create virtualenv
Setup->>Setup: Install dependencies
Setup->>Build: Dependencies ready
Build->>Build: Download models & test data
Build->>Build: Run qnn-onnx-converter<br/>(quantization pipeline)
Build->>Build: Build per-target libraries
Build->>Artifact: Conversion complete
Artifact->>Artifact: Collect per-target artifacts
Artifact->>Artifact: Generate tarballs<br/>(per-target dirs)
Artifact->>Release: Artifacts ready
alt Repository owner is csukuangfj or k2-fsa
Release->>Release: Release tar.bz2 packages<br/>to GitHub Releases
else Other owner
Release->>Release: Upload artifacts only
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring attention:
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds a new GitHub Actions workflow to export Zipformer CTC models to QNN (Qualcomm Neural Network) format for deployment on Qualcomm platforms. The workflow processes multiple duration configurations (5-30 seconds) of a Chinese Zipformer CTC ASR model and generates quantized libraries for both Linux x64 and Android aarch64 targets.
Key changes:
- Automated export pipeline for Zipformer CTC models to QNN format with INT8 quantization
- Matrix-based execution across 11 different input duration configurations (5-30 seconds)
- Dual-platform artifact generation (Linux x64 and Android aarch64) for each configuration
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: ${{ matrix.input_in_seconds }}-seconds | ||
| path: ./tmp/*.json |
There was a problem hiding this comment.
The artifact upload path ./tmp/*.json doesn't match the expected output. Based on the workflow steps, .json files are not generated. The tar.bz2 files are moved to the parent directory (../) at line 278, but no JSON files are created. Either:
- Remove this artifact upload if JSON files are not needed
- Update the path to match actual artifacts if JSON files should be generated
| path: ./tmp/*.json | |
| path: ./*.tar.bz2 |
![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 (1)
.github/workflows/export-zipformer-ctc-to-qnn-20250703.yaml (1)
216-216: Validate script path before execution.Line 216 runs
python3 ../scripts/pyannote/segmentation/show-onnx.pywith a hardcoded relative path. If the script does not exist, the step will fail without clear diagnostics. Additionally, the current directory istmp/(set on line 191), making the path fragile to directory structure changes.Consider adding a check or using a more explicit path:
+ if [[ ! -f ../scripts/pyannote/segmentation/show-onnx.py ]]; then + echo "Error: show-onnx.py script not found at ../scripts/pyannote/segmentation/show-onnx.py" + exit 1 + fi python3 ../scripts/pyannote/segmentation/show-onnx.py --filename ./model-$t-seconds.onnxAlternatively, define the script path as a variable at the beginning of the "Run" step for maintainability.
| - name: Install linux dependencies | ||
| shell: bash | ||
| run: | | ||
| ls -lh | ||
|
|
||
| echo "---" | ||
|
|
||
| ls -lh qairt | ||
|
|
||
| cd qairt/2.33.0.250327/bin | ||
| source envsetup.sh | ||
|
|
||
| yes | sudo ${QNN_SDK_ROOT}/bin/check-linux-dependency.sh || true |
There was a problem hiding this comment.
Reconsider error suppression in dependency check.
Line 87 uses yes | sudo ... || true, which suppresses all errors (both from the script and from sudo). If the dependency check legitimately fails, this masks the issue and allows the workflow to proceed with potentially missing dependencies.
Consider running the check without automatic suppression first, and only use || true if the script is expected to fail gracefully in certain conditions:
- yes | sudo ${QNN_SDK_ROOT}/bin/check-linux-dependency.sh || true
+ yes | sudo ${QNN_SDK_ROOT}/bin/check-linux-dependency.shIf the script is known to return non-zero even on success, add a comment explaining why suppression is necessary and consider checking the actual exit behavior.
📝 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.
| - name: Install linux dependencies | |
| shell: bash | |
| run: | | |
| ls -lh | |
| echo "---" | |
| ls -lh qairt | |
| cd qairt/2.33.0.250327/bin | |
| source envsetup.sh | |
| yes | sudo ${QNN_SDK_ROOT}/bin/check-linux-dependency.sh || true | |
| - name: Install linux dependencies | |
| shell: bash | |
| run: | | |
| ls -lh | |
| echo "---" | |
| ls -lh qairt | |
| cd qairt/2.33.0.250327/bin | |
| source envsetup.sh | |
| yes | sudo ${QNN_SDK_ROOT}/bin/check-linux-dependency.sh |
| for p in x86_64-linux-clang aarch64-android; do | ||
| if [[ $p == x86_64-linux-clang ]]; then | ||
| d=sherpa-onnx-qnn-$t-seconds-zipformer-ctc-zh-2025-07-03-int8-linux-x64 | ||
| elif [[ $p == aarch64-android ]]; then | ||
| d=sherpa-onnx-qnn-$t-seconds-zipformer-ctc-zh-2025-07-03-int8-android-aarch64 | ||
| else | ||
| echo "Unknown $p" | ||
| exit -1 | ||
| fi |
There was a problem hiding this comment.
Correct the exit code convention.
Line 256 uses exit -1, which is non-standard. The exit code -1 is reinterpreted as 255 by the shell. Use exit 1 for a standard error exit code.
else
echo "Unknown $p"
- exit -1
+ exit 1
fi🤖 Prompt for AI Agents
.github/workflows/export-zipformer-ctc-to-qnn-20250703.yaml around lines 249 to
257: the script uses `exit -1` which is non-standard (shell interprets negative
codes as 255); change the exit to a positive standard error code (for example
`exit 1`) so failures are signaled correctly and consistently.
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: ${{ matrix.input_in_seconds }}-seconds | ||
| path: ./tmp/*.json |
There was a problem hiding this comment.
Fix artifact upload path—workflow produces .tar.bz2 files, not .json files.
The artifact upload attempts to capture ./tmp/*.json, but the workflow generates *.tar.bz2 files (created in tmp/ and moved to the parent directory on line 278). No JSON files are produced, so this upload will find nothing.
Update the artifact path to match what the workflow actually produces:
- uses: actions/upload-artifact@v4
with:
name: ${{ matrix.input_in_seconds }}-seconds
- path: ./tmp/*.json
+ path: ./*.tar.bz2Alternatively, if .json files should be captured, clarify the intent and update the main build step accordingly.
📝 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/upload-artifact@v4 | |
| with: | |
| name: ${{ matrix.input_in_seconds }}-seconds | |
| path: ./tmp/*.json | |
| - uses: actions/upload-artifact@v4 | |
| with: | |
| name: ${{ matrix.input_in_seconds }}-seconds | |
| path: ./*.tar.bz2 |
🤖 Prompt for AI Agents
In .github/workflows/export-zipformer-ctc-to-qnn-20250703.yaml around lines 280
to 283 the artifact upload step is pointing at ./tmp/*.json while the workflow
actually produces .tar.bz2 archives (created in tmp/ and then moved to parent
dir on line 278); update the upload-artifact path to match the produced files
(for example point to the .tar.bz2 location or pattern such as ./*.tar.bz2 or
tmp/*.tar.bz2 depending on where files end up) or, if JSON artifacts were
intended, change the earlier build step to produce JSONs and keep the path
as-is.
| - name: Release | ||
| if: github.repository_owner == 'csukuangfj' | ||
| uses: svenstaro/upload-release-action@v2 | ||
| with: | ||
| file_glob: true | ||
| file: ./*.tar.bz2 | ||
| overwrite: true | ||
| repo_name: k2-fsa/sherpa-onnx | ||
| repo_token: ${{ secrets.UPLOAD_GH_SHERPA_ONNX_TOKEN }} | ||
| tag: asr-models-qnn | ||
|
|
||
| - name: Release | ||
| if: github.repository_owner == 'k2-fsa' | ||
| uses: svenstaro/upload-release-action@v2 | ||
| with: | ||
| file_glob: true | ||
| file: ./*.tar.bz2 | ||
| overwrite: true | ||
| tag: asr-models-qnn |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Deduplicate and clarify the two identical "Release" step names.
Both Release steps (lines 285–295 and 296–303) have the same name, which reduces clarity in the workflow UI and logs. Rename them to distinguish their purposes. Additionally, verify that the file path ./*.tar.bz2 correctly resolves after the "Run" step, since the tarball files are generated inside the tmp/ subdirectory and moved to the parent on line 278.
Apply this diff to rename the steps for clarity:
- name: Release
+ id: release-csukuangfj
if: github.repository_owner == 'csukuangfj'
uses: svenstaro/upload-release-action@v2
with:
file_glob: true
file: ./*.tar.bz2
overwrite: true
repo_name: k2-fsa/sherpa-onnx
repo_token: ${{ secrets.UPLOAD_GH_SHERPA_ONNX_TOKEN }}
tag: asr-models-qnn
- name: Release
+ id: release-k2fsa
if: github.repository_owner == 'k2-fsa'
uses: svenstaro/upload-release-action@v2
with:
file_glob: true
file: ./*.tar.bz2
overwrite: true
tag: asr-models-qnnCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/export-zipformer-ctc-to-qnn-20250703.yaml lines 285-303: both
Release steps use the identical name and possibly the wrong file glob; rename
each step to be distinct (e.g., "Release (csukuangfj)" and "Release (k2-fsa)")
so the workflow UI/logs clearly show which branch/publisher ran, and update the
uploaded file path to match where the tarballs actually exist after the Run step
(use tmp/*.tar.bz2 or ensure files are moved to the workspace root before using
./*.tar.bz2); keep the existing if conditions and repo/token settings.
You can find the exported models at
https://github.com/k2-fsa/sherpa-onnx/releases/tag/asr-models-qnn
The original onnx model is from
https://k2-fsa.github.io/sherpa/onnx/pretrained_models/offline-ctc/icefall/zipformer.html#sherpa-onnx-zipformer-ctc-zh-int8-2025-07-03-chinese
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.