Skip to content

Fix initializing fp32 Qwen3-ASR models.#3480

Merged
csukuangfj merged 1 commit into
k2-fsa:masterfrom
csukuangfj:fix-qwen3-asr-fp32
Apr 8, 2026
Merged

Fix initializing fp32 Qwen3-ASR models.#3480
csukuangfj merged 1 commit into
k2-fsa:masterfrom
csukuangfj:fix-qwen3-asr-fp32

Conversation

@csukuangfj

@csukuangfj csukuangfj commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Chores

    • Updated build configuration formatting and whitespace.
  • Refactor

    • Optimized model loading mechanism for offline Qwen3 ASR.

@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Apr 8, 2026
@csukuangfj csukuangfj merged commit 625f63a into k2-fsa:master Apr 8, 2026
15 of 27 checks passed
@coderabbitai

coderabbitai Bot commented Apr 8, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7ad15e47-33b8-4dbb-94f1-072c90a02e91

📥 Commits

Reviewing files that changed from the base of the PR and between 23185f8 and 43f46c9.

📒 Files selected for processing (2)
  • sherpa-onnx/csrc/CMakeLists.txt
  • sherpa-onnx/csrc/offline-qwen3-asr-model.cc

📝 Walkthrough

Walkthrough

Build configuration whitespace cleanup and refactoring of model initialization in Qwen3 ASR model to use path-based Ort::Session construction instead of file buffer reading, with corresponding updates to initialization functions to handle null input case.

Changes

Cohort / File(s) Summary
Build Configuration
sherpa-onnx/csrc/CMakeLists.txt
Removed extraneous blank lines in conditional sections and adjusted whitespace formatting in the executable list definition; no functional changes to conditions or contents.
Model Initialization
sherpa-onnx/csrc/offline-qwen3-asr-model.cc
Refactored model initialization to construct Ort::Session objects directly from model paths using SHERPA_ONNX_TO_ORT_PATH(...) instead of reading from file buffers. Updated InitConvFrontend, InitEncoder, and InitDecoder to handle nullptr/0 input case with error exit on uninitialized session. Added text-utils header include.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • #2964: Applies the same refactoring pattern to a different model file—adding text-utils include and switching to path-based SHERPA_ONNX_TO_ORT_PATH session construction.
  • #2788: Contains the same code-level changes (text-utils header addition and path-based Ort::Session initialization with Init(nullptr, 0) fallback) applied to other model implementations.

Suggested labels

size:L

Poem

🐰 A hop and a skip through paths so clear,
Sessions now bloom without buffers near,
Cleaner flows with error care,
CMake whitespace, crisp and fair!

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@csukuangfj csukuangfj deleted the fix-qwen3-asr-fp32 branch April 8, 2026 06:11

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the OfflineQwen3ASRModel to initialize ONNX sessions directly from file paths and updates the initialization logic to handle null model data. Feedback suggests restoring path validation to prevent potential crashes and using const void* for model data parameters to improve type safety.

Comment on lines +135 to +148
conv_sess_ = std::make_unique<Ort::Session>(
env_, SHERPA_ONNX_TO_ORT_PATH(c.conv_frontend), sess_opts_conv_);

InitConvFrontend(nullptr, 0);

encoder_sess_ = std::make_unique<Ort::Session>(
env_, SHERPA_ONNX_TO_ORT_PATH(c.encoder), sess_opts_encoder_);

InitEncoder(nullptr, 0);

decoder_sess_ = std::make_unique<Ort::Session>(
env_, SHERPA_ONNX_TO_ORT_PATH(c.decoder), sess_opts_decoder_);

InitDecoder(nullptr, 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The previous implementation used ReadFile which explicitly checked if the model paths were valid and if the files could be read. The new implementation directly creates Ort::Session from file paths. If a path is empty or the file is missing, Ort::Session will throw an exception, which might result in a crash if not caught, and the error message might be less specific than before. Consider adding explicit checks for empty paths to maintain robust error reporting.

    if (c.conv_frontend.empty()) {
      SHERPA_ONNX_LOGE("qwen3_asr.conv_frontend is empty");
      SHERPA_ONNX_EXIT(-1);
    }
    conv_sess_ = std::make_unique<Ort::Session>(
        env_, SHERPA_ONNX_TO_ORT_PATH(c.conv_frontend), sess_opts_conv_);

    InitConvFrontend(nullptr, 0);

    if (c.encoder.empty()) {
      SHERPA_ONNX_LOGE("qwen3_asr.encoder is empty");
      SHERPA_ONNX_EXIT(-1);
    }
    encoder_sess_ = std::make_unique<Ort::Session>(
        env_, SHERPA_ONNX_TO_ORT_PATH(c.encoder), sess_opts_encoder_);

    InitEncoder(nullptr, 0);

    if (c.decoder.empty()) {
      SHERPA_ONNX_LOGE("qwen3_asr.decoder is empty");
      SHERPA_ONNX_EXIT(-1);
    }
    decoder_sess_ = std::make_unique<Ort::Session>(
        env_, SHERPA_ONNX_TO_ORT_PATH(c.decoder), sess_opts_decoder_);

    InitDecoder(nullptr, 0);

@@ -224,8 +213,16 @@ class OfflineQwen3ASRModel::Impl {
}

void InitConvFrontend(void *model_data, size_t model_data_length) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The model_data parameter should be const void* as the buffer is only read to initialize the session.

  void InitConvFrontend(const void *model_data, size_t model_data_length) {

@@ -243,8 +240,16 @@ class OfflineQwen3ASRModel::Impl {
}

void InitEncoder(void *model_data, size_t model_data_length) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The model_data parameter should be const void* as the buffer is only read to initialize the session.

  void InitEncoder(const void *model_data, size_t model_data_length) {

@@ -267,8 +272,16 @@ class OfflineQwen3ASRModel::Impl {
}

void InitDecoder(void *model_data, size_t model_data_length) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The model_data parameter should be const void* as the buffer is only read to initialize the session.

  void InitDecoder(const void *model_data, size_t model_data_length) {

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

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant