支持openvino推理及增加openvino参数支持 #38
Conversation
|
感谢，待我晚些合并 |
There was a problem hiding this comment.
Pull request overview
该 PR 旨在为 rapid_layout 增加 OpenVINO 推理引擎支持，并提供对应的引擎参数配置入口。
Changes:
- 新增
EngineType.OPENVINO，并在get_engine()中接入 OpenVINO 引擎选择逻辑 - 新增 OpenVINO 推理会话实现
OpenVINOInferSession - 在
engine_cfg.yaml中新增openvino引擎配置项（device/threads/performance 等）
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| rapid_layout/utils/typings.py | 新增 OpenVINO 引擎枚举类型 |
| rapid_layout/inference_engine/openvino/main.py | 新增 OpenVINO 推理会话实现与运行时配置解析 |
| rapid_layout/inference_engine/openvino/init.py | 导出 OpenVINOInferSession |
| rapid_layout/inference_engine/base.py | get_engine() 增加 OpenVINO 分支 |
| rapid_layout/configs/engine_cfg.yaml | 增加 openvino 默认配置块 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _set(k, v, *, cast=str): | ||
| if v is not None and v != -1: | ||
| config[k] = cast(v) | ||
|
|
||
| _set("INFERENCE_NUM_THREADS", | ||
| engine_cfg.get("inference_num_threads", -1), | ||
| cast=lambda x: str(min(x, os.cpu_count())) if x > 0 else None) | ||
|
|
There was a problem hiding this comment.
_set() can write invalid values into the OpenVINO config: the cast lambda for INFERENCE_NUM_THREADS may return None, but _set() will still assign it (since it only checks the original v). This can result in {"INFERENCE_NUM_THREADS": None} being passed to compile_model, which is likely rejected. Consider only adding the entry if the casted value is not None (and/or do validation per key).
| val = self.model.get_rt_info()["framework"][key] | ||
| return val.value.splitlines() | ||
|
|
||
| def have_key(self, key: str = "character") -> bool: | ||
| try: | ||
| rt_info = self.model.get_rt_info() | ||
| return key in rt_info | ||
| except: |
There was a problem hiding this comment.
have_key() checks key in rt_info, but get_character_list() reads self.model.get_rt_info()["framework"][key]. These are inconsistent paths, so have_key() may return False even when get_character_list() would succeed (or vice versa). Align both methods to use the same rt_info structure (including the framework level) and avoid a bare except (catch a specific exception).
| val = self.model.get_rt_info()["framework"][key] | |
| return val.value.splitlines() | |
| def have_key(self, key: str = "character") -> bool: | |
| try: | |
| rt_info = self.model.get_rt_info() | |
| return key in rt_info | |
| except: | |
| rt_info = self.model.get_rt_info() | |
| framework_info = rt_info["framework"] | |
| val = framework_info[key] | |
| return val.value.splitlines() | |
| def have_key(self, key: str = "character") -> bool: | |
| try: | |
| rt_info = self.model.get_rt_info() | |
| framework_info = rt_info.get("framework", {}) | |
| return key in framework_info | |
| except (AttributeError, TypeError, KeyError): |
| elif engine_type == EngineType.OPENVINO: | ||
| if not import_package(engine_type.value): | ||
| raise ImportError(f"{engine_type.value} is not installed.") | ||
|
|
||
| from .openvino import OpenVINOInferSession | ||
|
|
||
| return OpenVINOInferSession |
There was a problem hiding this comment.
get_engine() now supports EngineType.OPENVINO, but there is no test coverage validating the new branch (e.g., that it returns OpenVINOInferSession when OpenVINO is available, and raises an ImportError when it is not). Adding a small unit test that mocks import_package() would prevent regressions without requiring OpenVINO to be installed in CI.
| openvino: | ||
| device: "CPU" | ||
| inference_num_threads: -1 | ||
| performance_hint: "LATENCY" | ||
| performance_num_requests: -1 | ||
| enable_cpu_pinning: null | ||
| num_streams: -1 | ||
| enable_hyper_threading: null | ||
| scheduling_core_type: null |
There was a problem hiding this comment.
The new openvino: block uses a different indentation style than the existing onnxruntime: block in the same YAML file. Aligning indentation within a single config file improves readability and reduces the chance of accidental structural mistakes during future edits.
| _set("INFERENCE_NUM_THREADS", | ||
| engine_cfg.get("inference_num_threads", -1), | ||
| cast=lambda x: str(min(x, os.cpu_count())) if x > 0 else None) | ||
|
|
There was a problem hiding this comment.
_init_config uses os.cpu_count() but os is never imported in this module, which will raise a NameError the first time OpenVINO is initialized. Import os and also guard against os.cpu_count() returning None when computing the thread limit.
| def _init_config(self, cfg: DictConfig) -> Dict[str, str]: | ||
| config = {} | ||
| engine_cfg = cfg.get("engine_cfg", {}) | ||
|
|
There was a problem hiding this comment.
_init_config reads settings from cfg.get("engine_cfg", {}), but the OpenVINO section in engine_cfg.yaml defines keys directly under openvino: (no nested engine_cfg). As written, all OpenVINO runtime options (threads, performance_hint, etc.) will be ignored. Use cfg directly (or pass the nested dict you expect) so the YAML keys actually take effect.
2 participants
支持openvino推理引擎