From 254ca199d1a104a2a4e414bf27f259e42bf41580 Mon Sep 17 00:00:00 2001 From: Baiju Meswani Date: Sat, 10 May 2025 00:20:31 -0700 Subject: [PATCH 1/4] Address QNN specific regressions --- build.py | 2 +- examples/python/model-qa.py | 6 +++--- src/config.cpp | 23 ++++++++++++++--------- src/generators.cpp | 5 +++-- src/models/input_ids.cpp | 5 +++++ src/models/model.cpp | 32 +++++++++++++++++++++++++------- 6 files changed, 51 insertions(+), 22 deletions(-) diff --git a/build.py b/build.py index ce96aeabd7..7143ae9eef 100644 --- a/build.py +++ b/build.py @@ -511,7 +511,7 @@ def update(args: argparse.Namespace, env: dict[str, str]): cuda_compiler = str(args.cuda_home / "bin" / "nvcc") command += [f"-DCMAKE_CUDA_COMPILER={cuda_compiler}"] - if util.is_windows(): + if args.package and util.is_windows(): command += _get_windows_build_args(args) if args.android: diff --git a/examples/python/model-qa.py b/examples/python/model-qa.py index faf84daf52..7d37a30109 100644 --- a/examples/python/model-qa.py +++ b/examples/python/model-qa.py @@ -150,9 +150,9 @@ def main(args): else: messages = f"""[{{"role": "system", "content": "{system_prompt}"}}, {{"role": "user", "content": "{text}"}}]""" # Apply Chat Template - final_prompt = tokenizer.apply_chat_template(messages=messages, add_generation_prompt=True) - final_input = tokenizer.encode(final_prompt) - generator.append_tokens(final_input) + prompt = tokenizer.apply_chat_template(messages=messages, add_generation_prompt=True) + input_tokens = tokenizer.encode(prompt) + generator.append_tokens(input_tokens) if args.verbose: print("Running generation loop ...") if args.timings: diff --git a/src/config.cpp b/src/config.cpp index 6ab5a4cd73..8f97b6c0d8 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -9,6 +9,17 @@ namespace Generators { +std::string NormalizeProviderName(std::string_view name) { + if (name == "qnn") { + return "QNN"; + } else if (name == "webgpu") { + return "WebGPU"; + } else if (name == "dml") { + return "DML"; + } + return std::string(name); +} + ONNXTensorElementDataType TranslateTensorType(std::string_view value) { if (value == "float32") { return ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT; @@ -72,13 +83,7 @@ struct ProviderOptionsArray_Element : JSON::Element { void OnComplete(bool /*empty*/) override { // For backwards compatibility turn our old names like 'qnn' into 'QNN', and 'webgpu' to 'WebGPU' for (auto& v : v_) { - if (v.name == "qnn") { - v.name = "QNN"; - } else if (v.name == "webgpu") { - v.name = "WebGPU"; - } else if (v.name == "dml") { - v.name = "DML"; - } + v.name = NormalizeProviderName(v.name); } } @@ -703,8 +708,8 @@ void ClearProviders(Config& config) { } void SetProviderOption(Config& config, std::string_view provider_name, std::string_view option_name, std::string_view option_value) { - if (!contains(config.model.decoder.session_options.providers, provider_name)) - config.model.decoder.session_options.providers.push_back(std::string(provider_name)); + if (auto normalized_provider = NormalizeProviderName(provider_name); !contains(config.model.decoder.session_options.providers, normalized_provider)) + config.model.decoder.session_options.providers.push_back(normalized_provider); std::ostringstream json; json << R"({")" << provider_name << R"(":{)"; diff --git a/src/generators.cpp b/src/generators.cpp index 7667521d25..c2a3956ace 100644 --- a/src/generators.cpp +++ b/src/generators.cpp @@ -359,8 +359,9 @@ void Generator::AppendTokens(cpu_span input_ids) { constexpr std::array devices_supporting_continuous_decoding{DeviceType::CPU, DeviceType::CUDA, DeviceType::WEBGPU}; if (search_->GetSequenceLength() != 0 && std::none_of(devices_supporting_continuous_decoding.begin(), devices_supporting_continuous_decoding.end(), - [this](DeviceType device_type) { return device_type == state_->params_->p_device->GetType(); })) - throw std::runtime_error("Continuous decoding is not supported on the selected device type (" + to_string(state_->params_->p_device->GetType()) + + [this](DeviceType device_type) { return device_type == state_->model_.p_device_kvcache_->GetType(); })) + // Support for continuous decoding should be based on the type of device used for KV cache + throw std::runtime_error("Continuous decoding is not supported on the selected device type (" + to_string(state_->model_.p_device_kvcache_->GetType()) + "). Please recreate the generator instance to avoid using continuous decoding."); if (last_action_ == Action::generated) { diff --git a/src/models/input_ids.cpp b/src/models/input_ids.cpp index 1badd4eff4..cb69d0059a 100644 --- a/src/models/input_ids.cpp +++ b/src/models/input_ids.cpp @@ -104,6 +104,11 @@ void DefaultInputIDs::Update(DeviceSpan new_tokens) { } WindowedInputIDs::WindowedInputIDs(State& state) : state_{state} { + if (model_.p_device_inputs_->GetType() != DeviceType::QNN && + model_.p_device_inputs_->GetType() != DeviceType::CPU) { + throw std::runtime_error("Sliding a window over input_ids requires works with only QNN and CPU providers."); + } + name_ = model_.config_->model.decoder.inputs.input_ids.c_str(); if (!model_.config_->model.decoder.sliding_window.has_value()) { diff --git a/src/models/model.cpp b/src/models/model.cpp index fcd0f3d6fa..2666d869a6 100644 --- a/src/models/model.cpp +++ b/src/models/model.cpp @@ -280,7 +280,16 @@ DeviceInterface* SetProviderSessionOptions(OrtSessionOptions& session_options, bool disable_graph_capture) { DeviceInterface* p_device{}; - for (auto& provider : providers) { + auto providers_list = providers; + if (!is_primary_session_options) { + // Providers specified in a non-primary provider options list are added + // to the primary providers. They are considered immutable and implcitly + // added as providers. + std::transform(provider_options_list.begin(), provider_options_list.end(), std::back_inserter(providers_list), + [](const auto& provider_options) { return provider_options.name; }); + } + + for (auto& provider : providers_list) { auto provider_options_it = std::find_if(provider_options_list.begin(), provider_options_list.end(), [&provider](const Config::ProviderOptions& po) { return po.name == provider; }); @@ -424,6 +433,11 @@ void EnsureDeviceOrtInit(DeviceInterface& device) { auto session_options = OrtSessionOptions::Create(); std::vector provider_options_list; provider_options_list.emplace_back(Config::ProviderOptions{device_type_names[static_cast(type)], {}}); + // QnnHtpShared is a special case. This allocator is only made available when the provider option + // 'enable_htp_shared_memory_allocator' is set to 1. + if (type == DeviceType::QNN) { + provider_options_list.back().options.emplace_back("enable_htp_shared_memory_allocator", "1"); + } const std::vector providers{device_type_names[static_cast(type)]}; SetProviderSessionOptions(*session_options, providers, provider_options_list, true, false); session_options->SetLogSeverityLevel(ORT_LOGGING_LEVEL_ERROR); // Errors only here, as warnings are not useful to the user @@ -622,13 +636,13 @@ void Model::CreateSessionOptionsFromConfig(const Config::SessionOptions& config_ session_options.SetGraphOptimizationLevel(config_session_options.graph_optimization_level.value()); } - p_device_ = SetProviderSessionOptions(session_options, config_session_options.providers, - config_session_options.provider_options, is_primary_session_options, - disable_graph_capture); + auto session_device = SetProviderSessionOptions(session_options, config_session_options.providers, + config_session_options.provider_options, is_primary_session_options, + disable_graph_capture); - // Fallback to CPU if no provider specific interface was set - if (!p_device_) - p_device_ = GetDeviceInterface(DeviceType::CPU); + if (!p_device_) { + p_device_ = session_device; + } } void Model::CreateSessionOptions() { @@ -642,6 +656,10 @@ void Model::CreateSessionOptions() { CreateSessionOptionsFromConfig(*pipeline_model.session_options, *emplaced.first->second, false, false); } } + + // Fallback to CPU if no provider specific interface was set + if (!p_device_) + p_device_ = GetDeviceInterface(DeviceType::CPU); } OrtSessionOptions* Model::GetSessionOptions(const std::string& model_id) const { From 614e55b395af12c362b68521ed9b6f850312526e Mon Sep 17 00:00:00 2001 From: Baiju Meswani Date: Sat, 10 May 2025 00:32:41 -0700 Subject: [PATCH 2/4] Update src/models/input_ids.cpp --- src/models/input_ids.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/input_ids.cpp b/src/models/input_ids.cpp index cb69d0059a..9333728963 100644 --- a/src/models/input_ids.cpp +++ b/src/models/input_ids.cpp @@ -106,7 +106,7 @@ void DefaultInputIDs::Update(DeviceSpan new_tokens) { WindowedInputIDs::WindowedInputIDs(State& state) : state_{state} { if (model_.p_device_inputs_->GetType() != DeviceType::QNN && model_.p_device_inputs_->GetType() != DeviceType::CPU) { - throw std::runtime_error("Sliding a window over input_ids requires works with only QNN and CPU providers."); + throw std::runtime_error("Sliding a window over input_ids works with either the QNN or the CPU provider."); } name_ = model_.config_->model.decoder.inputs.input_ids.c_str(); From 89a7cfa375e14faedd083893935f74e159d2788e Mon Sep 17 00:00:00 2001 From: Baiju Meswani Date: Sat, 10 May 2025 07:57:11 -0700 Subject: [PATCH 3/4] Update src/config.cpp Co-authored-by: Ryan Hill <38674843+RyanUnderhill@users.noreply.github.com> --- src/config.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index 8f97b6c0d8..06a172c849 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -9,7 +9,8 @@ namespace Generators { -std::string NormalizeProviderName(std::string_view name) { +// Fix casing of certain historical names to match current Onnxruntime names +std::string_view NormalizeProviderName(std::string_view name) { if (name == "qnn") { return "QNN"; } else if (name == "webgpu") { @@ -17,9 +18,8 @@ std::string NormalizeProviderName(std::string_view name) { } else if (name == "dml") { return "DML"; } - return std::string(name); + return name; // Return name unchanged } - ONNXTensorElementDataType TranslateTensorType(std::string_view value) { if (value == "float32") { return ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT; From 7826cf8111d0c8bdabb3db136044cffb8c7b7e31 Mon Sep 17 00:00:00 2001 From: Baiju Meswani Date: Sat, 10 May 2025 08:06:12 -0700 Subject: [PATCH 4/4] Address pull request review comments --- src/config.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.cpp b/src/config.cpp index 06a172c849..3a61120b8a 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -709,7 +709,7 @@ void ClearProviders(Config& config) { void SetProviderOption(Config& config, std::string_view provider_name, std::string_view option_name, std::string_view option_value) { if (auto normalized_provider = NormalizeProviderName(provider_name); !contains(config.model.decoder.session_options.providers, normalized_provider)) - config.model.decoder.session_options.providers.push_back(normalized_provider); + config.model.decoder.session_options.providers.push_back(std::string(normalized_provider)); std::ostringstream json; json << R"({")" << provider_name << R"(":{)";