已合并
【Feat】【Refactor】重构 collector 并适配 MindIE 和 CANN 新版本路径改动 #5011
刘家旺创建于 3月27日
【Feat】【Refactor】重构 collector 并适配 MindIE 和 CANN 新版本路径改动 #5011
已合并
刘家旺创建于 3月27日
共 4 个文件变更+999-40
@@ -1,4 +1,3 @@
1-# -*- coding: utf-8 -*-
2# -------------------------------------------------------------------------1# -------------------------------------------------------------------------
3# This file is part of the MindStudio project.2# This file is part of the MindStudio project.
4# Copyright (c) 2025-2026 Huawei Technologies Co.,Ltd.3# Copyright (c) 2025-2026 Huawei Technologies Co.,Ltd.
@@ -15,23 +14,28 @@
15# See the Mulan PSL v2 for more details.14# See the Mulan PSL v2 for more details.
16# -------------------------------------------------------------------------15# -------------------------------------------------------------------------
17 16 
17+import logging
18import os18import os
19-import shutil
20import platform19import platform
20+import shutil
21from abc import ABC, abstractmethod21from abc import ABC, abstractmethod
22+from typing import List, Optional, Dict
22 23 
23-from ..collectors import AscendCollector, LscpuCollector24+from ..core.strategy import Ascend, Lscpu
ChaseChe77
ChaseChe77ChaseChe773月27日

【Review】【正确性】 banner.py (line 23) 这里从 ..core.strategy 导入了 Ascend,但当前 PR 新增的 strategy.py 中并没有这个类定义。这样会导致 msprechecker.commands.banner 在导入阶段直接失败,banner 功能无法使用。建议先补齐 Ascend 策略实现,或者继续复用现有 AscendCollector,这样可以避免引入导入时崩溃。 【修改建议】

from ..collectors import AscendCollector, LscpuCollector

class AscendInfoSection(InfoSection):
    def get_info(self) -> str:
        ret = AscendCollector().collect()
        data = ret.data
        if not data:
            return "Ascend: not found"
        return self._format_ascend_info(data)

likedislike
刘家旺
刘家旺
3月27日 评论:
24-from ..utils import get_pkg_version, get_npu_type, get_npu_count, global_logger25+from ..util import get_npu_count, get_npu_memory, get_npu_type, get_pkg_version
26+ 
27+ 
28+logger = logging.getLogger(__name__)
25 29 
26 30 
27class InfoSection(ABC):31class InfoSection(ABC):
28 @abstractmethod32 @abstractmethod
29- def get_info(self):33+ def get_info(self) -> str:
30 pass34 pass
31 35 
32 36 
33class PlatformInfoSection(InfoSection):37class PlatformInfoSection(InfoSection):
34- def get_info(self):38+ def get_info(self) -> str:
35 return f"Platform: {platform.platform()}"39 return f"Platform: {platform.platform()}"
36 40 
37 41 
@@ -39,7 +43,7 @@ class PythonInfoSection(InfoSection):
39 def __init__(self, packages):43 def __init__(self, packages):
40 self.packages = packages44 self.packages = packages
41 45 
42- def get_info(self):46+ def get_info(self) -> str:
43 python_info = f"Python {platform.python_version()}"47 python_info = f"Python {platform.python_version()}"
44 for package in self.packages:48 for package in self.packages:
45 package_ver = get_pkg_version(package)49 package_ver = get_pkg_version(package)
@@ -51,73 +55,108 @@ class PythonInfoSection(InfoSection):
51 55 
52 56 
53class CpuInfoSection(InfoSection):57class CpuInfoSection(InfoSection):
54- def get_info(self):58+ def get_info(self) -> str:
55- ret = LscpuCollector().collect()59+ data = Lscpu().execute()
56- data = ret.data60+ model_name = (
57- model_name = data.get('model_name', 'Unknown Type')61+ data.get("Model name", "Unknown") if isinstance(data, dict) else "Unknown"
62+ )
58 return f"CPU: {model_name} ({os.cpu_count()} cores)"63 return f"CPU: {model_name} ({os.cpu_count()} cores)"
59 64 
60 65 
61class NpuInfoSection(InfoSection):66class NpuInfoSection(InfoSection):
62- def get_info(self):67+ def get_info(self) -> str:
63- npu_type, npu_device_nums = get_npu_type()68+ npu_type = get_npu_type()
64- npu_type = npu_type.display if npu_type else "Unknown Type"
65- npu_device_nums = npu_device_nums or 0
66 npu_count = get_npu_count()69 npu_count = get_npu_count()
67- return f"NPU: {npu_type} ({npu_device_nums} devices {npu_count} chips)"70+ npu_memory = get_npu_memory() # MB
71+ 
mjsz
mjszmjsz3月27日

【review】【正确性】get_npu_memory() 可能返回 None,当 npu_memory 为 None 时,npu_memory // 1024 会抛出 TypeError 异常。

代码建议:

npu_memory = get_npu_memory()  # MB
if npu_memory is None:
    return f"NPU: {npu_type.value} ({npu_count} devices)"
return f"NPU: {npu_type.value} ({npu_count} x {npu_memory // 1024}K)"

-- love from glm5

likedislike
刘家旺
刘家旺
3月27日 评论:
72+ if npu_memory is None:
73+ return f"NPU: {npu_type.value} ({npu_count})"
74+ return f"NPU: {npu_type.value} ({npu_count} x {npu_memory // 1024}G)"
刘家旺
刘家旺刘家旺3月27日

【review】【设计】 整除操作可能导致内存显示不准确,例如1536 MB会显示为1G而不是1.5G。建议优化为:

改动建议
74
- return f"NPU: {npu_type.value} ({npu_count} x {npu_memory // 1024}G)"
74
+ return f"NPU: {npu_type.value} ({npu_count} x {npu_memory / 1024:.1f}G)"
应用建议

这样可以保留一位小数,提高信息准确性。

likedislike
68 75 
69 76 
70class AscendInfoSection(InfoSection):77class AscendInfoSection(InfoSection):
71- def get_info(self):78+ def get_info(self) -> str:
72- ret = AscendCollector().collect()79+ data = Ascend().execute()
73- data = ret.data
74 if not data:80 if not data:
75 return "Ascend: not found"81 return "Ascend: not found"
76 return self._format_ascend_info(data)82 return self._format_ascend_info(data)
77 83 
78- def _format_ascend_info(self, data):84+ def _format_ascend_info(self, data) -> str:
79 prefix = "Ascend: "85 prefix = "Ascend: "
80 indent = " " * len(prefix)86 indent = " " * len(prefix)
81- items = list(data.items())
82 lines = []87 lines = []
83- for idx, (comp, info) in enumerate(items):88+ for idx, (comp, info) in enumerate(data.items()):
84 line_prefix = prefix if idx == 0 else indent89 line_prefix = prefix if idx == 0 else indent
85 lines.append(f"{line_prefix}{comp}: {self._format_version(info)}")90 lines.append(f"{line_prefix}{comp}: {self._format_version(info)}")
86 return "\n".join(lines)91 return "\n".join(lines)
87 92 
88- def _format_version(self, info):93+ def _format_version(self, info: Dict[str, str]) -> str:
89- comp_ver = info.get('version', "not found")94+ if not info:
90- timestamp = info.get('timestamp')95+ return "not found"
91- if timestamp:96+ 
92- comp_ver += f" ({timestamp})"97+ version = None
93- commit_id = info.get('commit')98+ timestamp = None
99+ commit_id = None
100+ for key in info:
101+ key_lower = key.lower()
102+ if "version" in key_lower: # version choose the last match
103+ version = info[key]
104+ elif commit_id is None and "commit" in key_lower:
105+ commit_id = info[key]
106+ elif timestamp is None and key_lower in {"time", "timestamp"}:
107+ timestamp = info[key]
108+ 
109+ if version is None:
110+ version = "not found"
111+ 
112+ comp_ver = f"{version} ({timestamp})" if timestamp else version
113+ 
94 if commit_id:114 if commit_id:
mjsz
mjszmjsz3月27日

【review】【正确性】_format_version 方法声明返回 str,但当 info 不包含任何带有 "version" 的 key 时,version 变量保持为 None,最后返回 None,与类型注解不符。

代码建议:

def _format_version(self, info) -> str:
    if not info:
        return "not found"

    version = None
    commit_id = None
    for key in info:
        if version is None and "version" in key.lower():
            version = info[key]
        elif commit_id is None and "commit" in key.lower():
            commit_id = info[key]

    if version is None:
        version = "not found"
    if commit_id:
        version += f" -- {commit_id}"

    return version

-- love from glm5

likedislike
刘家旺
刘家旺
3月27日 评论:
95 comp_ver += f" -- {commit_id}"115 comp_ver += f" -- {commit_id}"
116+ 
96 return comp_ver117 return comp_ver
97 118 
98 119 
99class BannerPresenter:120class BannerPresenter:
100- PYTHON_INFO_PACKAGES = ['msprechecker', 'torch', 'torch_npu', 'transformers']121+ TITLE = "MindStudio Prechecker Tool"
122+ PYTHON_INFO_PACKAGES = [
123+ "msprechecker",
124+ "torch",
125+ "torch_npu",
126+ "transformers",
127+ ]
128+ 
129+ def __init__(
130+ self,
131+ *,
132+ sections: Optional[List[InfoSection]] = None,
133+ python_packages: Optional[List[str]] = None,
134+ ):
135+ packages = python_packages or self.PYTHON_INFO_PACKAGES
刘家旺刘家旺
刘家旺刘家旺3月27日

此条代码评论区间+133至+135

【review】【设计】 参数 python_packages 的类型提示错误,应为字符串列表而非 InfoSection 实例列表。同时,直接使用类属性 PYTHON_INFO_PACKAGES 的引用可能导致意外修改,建议创建副本以避免副作用。

改动建议
133
133
  python_packages: Optional[List[str]] = None,
134
134
  ):
135
- packages = python_packages or self.PYTHON_INFO_PACKAGES
135
+ packages = list(python_packages) if python_packages is not None else list(self.PYTHON_INFO_PACKAGES)
应用建议

此修改确保类型安全并防止对类属性的意外更改,提高代码的健壮性和可维护性。

likedislike
刘家旺刘家旺3月28日

【review】【设计】 使用 or 操作符赋值可能导致 packages 引用类属性列表,后续 extend 会永久修改默认列表,影响所有实例。建议优化为:

改动建议
135
- packages = python_packages or self.PYTHON_INFO_PACKAGES
135
+ packages = list(python_packages) if python_packages is not None else self.PYTHON_INFO_PACKAGES[:]
应用建议

复制列表可避免副作用,确保实例独立性。

likedislike
136+ 
137+ # detect if it's mindie 3.0.0 by verifying if it is installed
138+ if get_pkg_version("mindie-llm") is not None:
139+ packages.extend(("mindie-llm", "mindie-motor", "atb-llm"))
101 140 
102- def __init__(self, *, sections=None):
103 self.sections = sections or [141 self.sections = sections or [
104 PlatformInfoSection(),142 PlatformInfoSection(),
105- PythonInfoSection(self.PYTHON_INFO_PACKAGES),143+ PythonInfoSection(packages),
106 CpuInfoSection(),144 CpuInfoSection(),
107 NpuInfoSection(),145 NpuInfoSection(),
108- AscendInfoSection()146+ AscendInfoSection(),
109 ]147 ]
110 148 
111 def add_section(self, section: InfoSection):149 def add_section(self, section: InfoSection):
112 self.sections.append(section)150 self.sections.append(section)
113- 151+ 
114- def print_banner(self):152+ def render(self) -> str:
153+ """Return the full banner as a string (enables testing and logging)."""
115 cols, _ = shutil.get_terminal_size()154 cols, _ = shutil.get_terminal_size()
116- 155+ lines = [f" {self.TITLE} ".center(cols, "=")]
117- title = "MindStudio Prechecker Tool"
118- global_logger.info(f" {title} ".center(cols, "="))
119- 
120 for section in self.sections:156 for section in self.sections:
121- global_logger.info(section.get_info())157+ lines.append(section.get_info())
158+ lines.append("-" * cols)
159+ return "\n".join(lines)
122 160 
123- global_logger.info("-" * cols)161+ def print_banner(self):
162+ print(self.render())
T
Tttcool3月27日

[review] [建议] 第 156-163 行 render() 方法新增后,print_banner() 改为调用 print(self.render()),这是一个很好的重构(职责分离、便于测试);但原来使用 global_logger.info(...) 的日志行为被完全替换成 print,若调用方依赖日志系统捕获 banner 内容,行为已发生静默变化,建议在 print_banner 或调用处保留一次 logger.info() 记录。

likedislike
刘家旺
刘家旺
3月27日 评论:
@@ -0,0 +1,60 @@
1+# -------------------------------------------------------------------------
2+# This file is part of the MindStudio project.
3+# Copyright (c) 2025-2026 Huawei Technologies Co.,Ltd.
4+#
5+# MindStudio is licensed under Mulan PSL v2.
6+# You can use this software according to the terms and conditions of the Mulan PSL v2.
7+# You may obtain a copy of Mulan PSL v2 at:
8+#
9+# `http://license.coscl.org.cn/MulanPSL2`
10+#
11+# THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND,
12+# EITHER EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO NON-INFRINGEMENT,
13+# MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE.
14+# See the Mulan PSL v2 for more details.
15+# -------------------------------------------------------------------------
16+ 
17+import logging
18+from typing import Any, Dict, List, Optional
19+ 
20+from .strategy import CollectStrategy
21+ 
22+ 
23+logger = logging.getLogger(__name__)
24+ 
25+ 
26+class Collector:
27+ def __init__(
28+ self, collect_strategies: Optional[List[CollectStrategy]] = None
29+ ) -> None:
30+ self._collect_strategies: List[CollectStrategy] = []
31+ if collect_strategies is not None:
32+ try:
33+ self._collect_strategies = list(collect_strategies)
34+ except TypeError:
35+ logger.error(
36+ "collect_strategies must be an iterable. Got %s instead",
37+ collect_strategies,
38+ )
39+ raise
T
Tttcool3月27日

[review] [建议] Collector.init 中捕获了 TypeError 并 raise,但同一 init 的后续类型检查(第 213-219 行)却未做类似保护。两段校验逻辑风格不统一:前者捕获异常再抛,后者直接 raise——建议统一为直接 raise,去掉 try/except 层,保持简洁: if collect_strategies is not None: self._collect_strategies = list(collect_strategies) # 若不可迭代自然抛 TypeError

likedislike
刘家旺
刘家旺
3月27日 评论:
40+ 
41+ if not all(
42+ isinstance(strategy, CollectStrategy)
43+ for strategy in self._collect_strategies
44+ ):
45+ raise TypeError(
46+ "All collect_strategies must be instances of CollectStrategy"
47+ )
48+ 
49+ def add_strategy(self, *, collect_strategy: CollectStrategy) -> None:
50+ if not isinstance(collect_strategy, CollectStrategy):
51+ raise TypeError(
52+ f"collect_strategy must be an instance of CollectStrategy, got {type(collect_strategy)!r} instead"
53+ )
54+ 
55+ self._collect_strategies.append(collect_strategy)
56+ 
57+ def collect(self) -> Dict[str, Any]:
ChaseChe77
ChaseChe77ChaseChe773月27日

【review】【设计】 这里 Collector.collect() 使用 dict comprehension 直接执行所有策略,只要任意一个 strategy.execute() 抛异常,整个采集流程就会中断;但同文件系里的 CollectStrategyGroup.execute() 已经采用了“单策略失败不影响整体”的容错语义。建议两者保持一致,否则后续调用方很难稳定依赖这套新抽象。 【修改建议】

def collect(self) -> Dict[str, Any]:
    results: Dict[str, Any] = {}
    for strategy in self._collect_strategies:
        try:
            results[strategy.name] = strategy.execute()
        except Exception:
            logger.exception("Strategy %r failed", strategy.name)
            results[strategy.name] = None
    return results

likedislike
58+ return {
59+ strategy.name: strategy.execute() for strategy in self._collect_strategies
60+ }
@@ -0,0 +1,149 @@
1+# -------------------------------------------------------------------------
2+# This file is part of the MindStudio project.
3+# Copyright (c) 2025-2026 Huawei Technologies Co.,Ltd.
4+#
5+# MindStudio is licensed under Mulan PSL v2.
6+# You can use this software according to the terms and conditions of the Mulan PSL v2.
mjsz
mjszmjsz3月27日

【review】【资料】第6行注释中存在拼写错误 "terms and conditions of" 应为 "terms and conditions of the"。

代码建议:

# You can use this software according to the terms and conditions of the Mulan PSL v2.
likedislike
刘家旺
刘家旺
3月27日 评论:
7+# You may obtain a copy of Mulan PSL v2 at:
8+#
9+# `http://license.coscl.org.cn/MulanPSL2`
10+#
11+# THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND,
12+# EITHER EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO NON-INFRINGEMENT,
13+# MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE.
14+# See the Mulan PSL v2 for more details.
15+# -------------------------------------------------------------------------
16+ 
17+import itertools
18+import logging
19+import os
20+import re
21+import shutil
22+import stat
23+import subprocess
24+from enum import Enum
25+from typing import Optional
26+ 
27+try:
28+ from importlib.metadata import PackageNotFoundError, version
29+except ImportError:
30+ from importlib_metadata import PackageNotFoundError, version
31+ 
32+from packaging.version import InvalidVersion, Version
33+ 
34+ 
35+LOG_LEVELS = {
36+ "debug": logging.DEBUG,
37+ "info": logging.INFO,
38+ "warning": logging.WARNING,
39+ "error": logging.ERROR,
40+ "critical": logging.CRITICAL,
41+}
42+LOG_FORMAT = "%(levelname)-5s %(asctime)s [%(filename)s:%(lineno)d] %(message)s"
43+ 
44+ 
45+logger = logging.getLogger(__name__)
46+ 
47+ 
48+class NpuType(Enum):
49+ d100 = "300"
50+ d500 = "300I_DUO"
51+ d801 = "800I A1"
52+ d802 = "800I A2"
53+ d803 = "800I A3"
54+ UNKNOWN = "unknown"
55+ 
56+ 
57+class Framework(Enum):
58+ MINDIE = "mindie"
59+ VLLM = "vllm"
60+ SGLANG = "sglang"
61+ UNKNOWN = "unknown"
62+ 
63+ 
64+def get_pkg_version(pkg_name: str) -> Optional[Version]:
mjsz
mjszmjsz3月27日

【review】【正确性】Version() 构造函数可能抛出 packaging.version.InvalidVersion 异常(当版本字符串格式无效时),当前代码只捕获了 PackageNotFoundError,可能导致未处理的异常。

代码建议:

from packaging.version import InvalidVersion, Version

def get_pkg_version(pkg_name: str) -> Optional[Version]:
    try:
        return Version(version(pkg_name))
    except (PackageNotFoundError, InvalidVersion):
        return None

-- love from glm5

likedislike
刘家旺
刘家旺
3月27日 评论:
65+ try:
66+ pkg_version = version(pkg_name)
67+ except PackageNotFoundError:
68+ return None
69+ 
70+ try:
71+ return Version(pkg_version)
72+ except InvalidVersion:
73+ logger.warning(
74+ "Got invalid version '%s' from package '%s'", pkg_version, pkg_name
75+ )
76+ return None
77+ 
78+ 
79+def get_npu_count() -> int:
80+ """Count Davinci NPU character devices present on the machine."""
81+ template = "/dev/davinci{}"
82+ for device_id in itertools.count(0):
83+ try:
84+ mode = os.stat(template.format(device_id)).st_mode
85+ except OSError:
86+ # Device path doesn't exist: we've counted all devices.
mjsz
mjszmjsz3月27日

【review】【可维护性】第91行注释中存在拼写错误 "doesn't" 应为 "doesn't"。

代码建议:

# Device path doesn't exist: we've counted all devices.
likedislike
刘家旺
刘家旺
3月27日 评论:
87+ return device_id
88+ if not stat.S_ISCHR(mode):
89+ # Path exists but is not a character device: stop here.
90+ return device_id
91+ return 0 # unreachable; satisfies type checkers
92+ 
93+ 
94+def get_npu_type() -> NpuType:
95+ """Detect NPU type via lspci accelerator entries."""
96+ lspci = shutil.which("lspci")
97+ if not lspci:
98+ return NpuType.UNKNOWN
99+ 
100+ try:
101+ output = subprocess.check_output([lspci], stderr=subprocess.DEVNULL, text=True)
102+ except Exception:
103+ logger.exception("Failed to execute lspci")
104+ return NpuType.UNKNOWN
105+ 
106+ device_pattern = re.compile(r"device\s*(\w+)", re.IGNORECASE)
107+ devices = []
108+ for line in output.splitlines():
109+ if "accelerator" not in line.lower():
110+ continue
111+ m = device_pattern.search(line)
112+ if m:
113+ devices.append(m.group(1))
114+ 
115+ if not devices:
116+ return NpuType.UNKNOWN
117+ 
118+ first = devices[0]
119+ if first not in NpuType.__members__ or any(d != first for d in devices):
120+ logger.debug("Inconsistent or unrecognised device types: %s", devices)
121+ return NpuType.UNKNOWN
122+ 
mjsz
mjszmjsz3月27日

【review】【正确性】get_npu_type 函数中,NpuType.__members__ 包含的是枚举成员名(如 d100, d500),而 first 是从 lspci 输出中提取的设备标识符(如 d100, d801),两者格式可能不完全匹配。建议检查匹配逻辑是否正确。

代码建议:

# 如果 lspci 输出的设备标识符格式为 "d100", "d801" 等,当前逻辑是正确的
# 如果格式不同,需要调整匹配逻辑,例如:
first_upper = first.upper()
if first_upper not in NpuType.__members__ or any(d != first for d in devices):
    logger.debug("Inconsistent or unrecognised device types: %s", devices)
    return NpuType.UNKNOWN

return NpuType[first_upper]

-- love from glm5

likedislike
刘家旺
刘家旺
3月27日 评论:
123+ return NpuType[first]
124+ 
125+ 
126+def get_npu_memory() -> Optional[int]:
T
Tttcool3月27日

[review] [规范] 第 77 行 get_npu_memory() 返回值类型为 Optional[int],但注释写的是 # MB,而 NpuInfoSection.get_info() 中直接做整除 npu_memory // 1024 时若 npu_memory 为 None 会抛出 TypeError;修改建议:添加 None 保护,参考代码如下: def get_info(self) -> str: npu_type = get_npu_type() npu_count = get_npu_count() npu_memory = get_npu_memory() memory_str = f"{npu_memory // 1024}K" if npu_memory is not None else "Unknown" return f"NPU: {npu_type.value} ({npu_count} x {memory_str})"

likedislike
刘家旺
刘家旺
3月27日 评论:
127+ """Return High-Bandwidth Memory capacity (MB) for device 0, or None if unavailable."""
128+ npu_smi = shutil.which("npu-smi")
129+ if not npu_smi:
130+ return None
131+ 
132+ try:
133+ output = subprocess.check_output(
134+ [npu_smi, "info", "-i", "0", "-t", "memory"],
135+ stderr=subprocess.DEVNULL,
136+ text=True,
137+ )
138+ except Exception:
139+ logger.exception("Failed to execute npu-smi")
140+ return None
141+ 
142+ for line in output.splitlines():
ChaseChe77
ChaseChe77ChaseChe773月27日

【Review】【正确性】 这里解析 npu-smi 输出时用了 value.isdigit(),但 HBM Capacity 的值通常会带单位,例如 65536 MB。这种情况下当前实现会一直返回 None,导致 NPU 内存信息无法正确展示。建议改成提取数值部分,这样兼容真实命令输出。 【修改建议】

for line in output.splitlines():
    if "HBM Capacity" not in line:
        continue
    match = re.search(r"(\d+)", line)
    if match:
        return int(match.group(1))

likedislike
143+ if "HB" + "M Capacity" not in line or ":" not in line:
144+ continue
145+ value = line.split(":")[-1].strip()
146+ if value.isdigit():
147+ return int(value)
148+ 
149+ return None