Upload DPDFNet models - #3322
Conversation
dosubot
Bot
added
the
size:S
📝 WalkthroughWalkthroughAdded a new workflow step to download DPDFNet ONNX models from Hugging Face, rename baseline files, and introduced a gated (disabled) Release block for uploading ONNX files to the sherpa-onnx repository while preserving the existing tarball upload workflow. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/upload-models.yaml (2)
1163-1172: Remove or document the disabled Release block.Static analysis flagged this
if: falsecondition. If this block is for future use, consider either:
- Removing it until needed (can be added back via PR when required)
- Adding a comment explaining why it's kept disabled
Additionally, having two steps named "Release" will cause confusion in workflow logs.
♻️ Option: Remove the disabled block entirely
- - - name: Release - if: false - uses: svenstaro/upload-release-action@v2 - with: - file_glob: true - file: ./*.onnx - overwrite: true - repo_name: k2-fsa/sherpa-onnx - repo_token: ${{ secrets.UPLOAD_GH_SHERPA_ONNX_TOKEN }} - tag: speech-enhancement-models♻️ Option: Document and rename if keeping for reference
+ # TODO: Enable this step to upload ONNX models to speech-enhancement-models release + # when ready for production use - - name: Release + - name: Release ONNX models (disabled) if: false uses: svenstaro/upload-release-action@v2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/upload-models.yaml around lines 1163 - 1172, The workflow contains a disabled release step named "Release" using svenstaro/upload-release-action@v2 with if: false and tag: speech-enhancement-models; either remove this entire block to avoid dead code and duplicate step names, or document why it is kept disabled (add an inline comment near if: false) and rename the step to a unique identifier (e.g., "Release (disabled - reference)") to avoid confusion in logs while preserving the action and inputs for future use.
27-42: Add error handling to wget downloads.The
wgetcommands will not fail the workflow on HTTP errors (e.g., 404, 500) without explicit flags. If a model file is unavailable, wget may save an HTML error page instead, leading to corrupted uploads or confusing downstream failures.Consider adding
set -eand usingwget --failorwget -q --show-progresswith explicit exit code checks.♻️ Proposed fix for robustness
- name: Upload DPDFNet shell: bash run: | + set -e models=( baseline.onnx dpdfnet2.onnx dpdfnet2_48khz_hr.onnx dpdfnet4.onnx dpdfnet8.onnx ) for m in ${models[@]}; do - wget https://huggingface.co/Ceva-IP/DPDFNet/resolve/main/onnx/$m + wget --retry-connrefused --waitretry=1 --tries=3 -q --show-progress \ + "https://huggingface.co/Ceva-IP/DPDFNet/resolve/main/onnx/$m" || exit 1 done mv baseline.onnx dpdfnet_baseline.onnx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/upload-models.yaml around lines 27 - 42, The current download loop using the models array and wget in the upload workflow can silently succeed with HTML error pages; update the script to enable strict failure and make wget fail on HTTP errors by adding a failing shell option (e.g., set -e) at the top of the run block and using wget --fail (or -q --show-progress --fail) inside the for m in ${models[@]} loop, and after each download check the exit status (or rely on set -e) so any missing file causes the job to fail early; ensure mv baseline.onnx dpdfnet_baseline.onnx only runs after successful download of baseline.onnx.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/upload-models.yaml:
- Around line 1163-1172: The workflow contains a disabled release step named
"Release" using svenstaro/upload-release-action@v2 with if: false and tag:
speech-enhancement-models; either remove this entire block to avoid dead code
and duplicate step names, or document why it is kept disabled (add an inline
comment near if: false) and rename the step to a unique identifier (e.g.,
"Release (disabled - reference)") to avoid confusion in logs while preserving
the action and inputs for future use.
- Around line 27-42: The current download loop using the models array and wget
in the upload workflow can silently succeed with HTML error pages; update the
script to enable strict failure and make wget fail on HTTP errors by adding a
failing shell option (e.g., set -e) at the top of the run block and using wget
--fail (or -q --show-progress --fail) inside the for m in ${models[@]} loop, and
after each download check the exit status (or rely on set -e) so any missing
file causes the job to fail early; ensure mv baseline.onnx dpdfnet_baseline.onnx
only runs after successful download of baseline.onnx.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 194aebbc-fe68-4cb8-83de-402217c12733
📒 Files selected for processing (1)
.github/workflows/upload-models.yaml
|
@csukuangfj Thanks for the quick support! I have one more question: when I click on Speech Enhancement, it takes me to Is there anything I need to do to get it listed on the documentation page as well, Thanks a lot! |
|
2 participants
See also
https://github.com/ceva-ip/DPDFNet
You can download models from
https://github.com/k2-fsa/sherpa-onnx/releases/tag/speech-enhancement-models
See also #3276
cc @danielr-ceva
16 kHz models
48 kHz model
Summary by CodeRabbit