From d20692dd3b5efdbbd5c9f4e5a1ce4d1b53e78015 Mon Sep 17 00:00:00 2001 From: Jackie2049 Date: Sat, 27 Jun 2026 23:27:57 +0800 Subject: [PATCH] =?UTF-8?q?[chore]=20=E5=BB=BA=E7=AB=8B=20CPU=20CI?= =?UTF-8?q?=EF=BC=9Aruff=20gate=20+=20=E5=85=A8=E9=87=8F=E6=B5=8B=E8=AF=95?= =?UTF-8?q?=20+=20coverage=20PR=20=E8=AF=84=E8=AE=BA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub 无可用 NPU/GPU runner，CI 只做 CPU 能覆盖的部分；NPU/GPU/verl080-e2e 测试靠 importorskip 自动跳过，由公司内 NPU 机在发版前手动跑。 - .github/workflows/ci.yml：push/PR/workflow_dispatch 触发，ubuntu + py3.9 + torch-CPU - ruff gate（F,E9 真 bug），format 仅 advisory - pytest tests/ 全量（importorskip 自动筛环境） - coverage 进 job summary + PR 评论（聚焦本次改动文件，单评论更新） - import smoke：prefix_sharing 与 setup.install 可导入 - pyproject.toml：[tool.ruff] select F,E9；[tool.coverage] omit tools/patches （核心覆盖率 75%，不被 0% 的 tools/patches 拖到误导性 34%） - requirements-ci.txt + .pre-commit-config.yaml（commit 前 ruff --fix） - .gitignore：补 coverage 产物 顺带修掉 F,E9 暴露的 27 个问题： - verl_mcore.py：restore_via_2d_unfold_verl080 里死变量 device（F841） - 26 个：F401 未用 import / F541 空 f-string（ruff --fix） （注：megatron_runtime.py 的 prefix_log 潜伏 NameError 已由 #37 一并清理） 本地验证：209 passed / 29 skipped，ruff F,E9 clean，核心 coverage 75%。 Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 110 ++++++++++++++++++ .gitignore | 6 + .pre-commit-config.yaml | 12 ++ .../backends/flash_atten_base.py | 1 - prefix-sharing/prefix_sharing/core/config.py | 12 +- .../prefix_sharing/integrations/verl_mcore.py | 29 ++--- .../prefix_sharing/setup/registry.py | 2 +- .../prefix_sharing/tools/cmp_diag.py | 3 +- .../prefix_sharing/tools/training_monitor.py | 2 +- prefix-sharing/pyproject.toml | 33 ++++++ prefix-sharing/requirements-ci.txt | 6 + 11 files changed, 188 insertions(+), 28 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 .pre-commit-config.yaml create mode 100644 prefix-sharing/requirements-ci.txt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..9aa5e0c2 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,110 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + workflow_dispatch: {} # 允许手动触发，便于首次/分支上验证 workflow + +permissions: + contents: read + pull-requests: write + +# Cancel superseded runs on the same ref (cheap CPU cycles matter). +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + name: lint + unit + torch-CPU integrated + runs-on: ubuntu-latest + defaults: + run: + working-directory: prefix-sharing + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.9" + cache: pip + cache-dependency-path: prefix-sharing/requirements-ci.txt + + - name: Install deps + run: | + python -m pip install --upgrade pip + # CPU-only torch: the 44 torch-using tests (restore autograd, torch_ref + # backend, TP/SP/PP layout) don't need CUDA. NPU/GPU/verl080-e2e tests + # self-skip via importorskip. + pip install torch --index-url https://download.pytorch.org/whl/cpu + pip install -r requirements-ci.txt + + - name: Ruff (gate — pyflakes + syntax errors) + run: ruff check --select F,E9 prefix_sharing + + - name: Ruff format (advisory — not yet a gate) + continue-on-error: true + run: ruff format --check prefix_sharing + + - name: Import smoke + run: | + python -c "import prefix_sharing; print('prefix_sharing import OK')" + python -c "from prefix_sharing.setup import install; print('setup.install importable')" + + - name: Test + coverage + run: | + coverage run -m pytest tests/ + coverage xml + coverage report | tee coverage-report.txt + + - name: Coverage → job summary + if: always() + run: | + { + echo '## 📊 Coverage (core; tools/patches omitted)' + echo + echo '```' + tail -n 5 coverage-report.txt + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + - name: Coverage PR comment + if: github.event_name == 'pull_request' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + set -e + # Focus: changed package files in this PR. + CHANGED=$(gh pr view "$PR_NUMBER" --json files \ + --jq '.files[].path' \ + | grep '^prefix-sharing/prefix_sharing/.*\.py$' \ + | sed 's#^prefix-sharing/##' \ + | paste -sd, - || true) + BODY="$(mktemp)" + { + echo "## 📊 Coverage report" + echo + echo '```' + coverage report | tail -n 4 + echo '```' + if [ -n "$CHANGED" ]; then + echo + echo "### Changed files in this PR" + echo '```' + coverage report --include="$CHANGED" || echo "(coverage: none of the changed files were imported by the test run)" + echo '```' + fi + } > "$BODY" + # Update existing comment if present (keeps the thread to one comment per PR). + EXISTING=$(gh pr view "$PR_NUMBER" --json comments \ + --jq '.comments[] | select(.body | contains("📊 Coverage report")) | .databaseId' \ + | head -n 1) + if [ -n "$EXISTING" ]; then + gh pr comment "$PR_NUMBER" --edit "$EXISTING" --body-file "$BODY" + else + gh pr comment "$PR_NUMBER" --body-file "$BODY" + fi diff --git a/.gitignore b/.gitignore index bb396e24..bf386282 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,9 @@ log/ # Local tool dirs .codebuddy/ + +# coverage artifacts +*,cover +coverage.xml +htmlcov/ +.coverage.* diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..13521ebe --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,12 @@ +# Local pre-commit hooks. Install with: `pre-commit install` +# +# Kept minimal on first introduction: only the CI gate rules auto-fix on commit +# (unused imports, undefined names, syntax errors). `ruff format` is intentionally +# NOT hooked yet — the codebase has never been auto-formatted, so enabling it +# would produce a noisy first-run diff. Enable once a format pass lands. +repos: + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.6.9 + hooks: + - id: ruff + args: [--select, F,E9, --fix] diff --git a/prefix-sharing/prefix_sharing/backends/flash_atten_base.py b/prefix-sharing/prefix_sharing/backends/flash_atten_base.py index 6147d649..e32a1ac1 100644 --- a/prefix-sharing/prefix_sharing/backends/flash_atten_base.py +++ b/prefix-sharing/prefix_sharing/backends/flash_atten_base.py @@ -14,7 +14,6 @@ from prefix_sharing.backends.base import BackendCapabilities from prefix_sharing.backends.packed_layout import PackedBatchLayout -from prefix_sharing.core.config import PrefixSharingConfig from prefix_sharing.core.planner import PrefixSharingPlan diff --git a/prefix-sharing/prefix_sharing/core/config.py b/prefix-sharing/prefix_sharing/core/config.py index 6d85047c..7aee42f8 100644 --- a/prefix-sharing/prefix_sharing/core/config.py +++ b/prefix-sharing/prefix_sharing/core/config.py @@ -183,15 +183,15 @@ def validate(self, model_config: Any | None = None, integrate_mode: str | None = ) if not self.supported_rope_fusion and rope_fusion: raise PrefixSharingConfigError( - f"[Config Error] apply_rope_fusion=True 不支持当前阶段。" - f"Phase 1 要求关闭 rope fusion (apply_rope_fusion=False)，" - f"请修改配置或禁用 prefix sharing。" + "[Config Error] apply_rope_fusion=True 不支持当前阶段。" + "Phase 1 要求关闭 rope fusion (apply_rope_fusion=False)，" + "请修改配置或禁用 prefix sharing。" ) if not self.supported_fused_qkv_rope and fused_qkv_rope: raise PrefixSharingConfigError( - f"[Config Error] fused_single_qkv_rope=True 不支持当前阶段。" - f"Phase 1 要求关闭 fused QKV rope (fused_single_qkv_rope=False)，" - f"请修改配置或禁用 prefix sharing。" + "[Config Error] fused_single_qkv_rope=True 不支持当前阶段。" + "Phase 1 要求关闭 fused QKV rope (fused_single_qkv_rope=False)，" + "请修改配置或禁用 prefix sharing。" ) if self.model_type == "text_only_causal_lm" and model_type != "text_only_causal_lm": raise PrefixSharingConfigError( diff --git a/prefix-sharing/prefix_sharing/integrations/verl_mcore.py b/prefix-sharing/prefix_sharing/integrations/verl_mcore.py index 7412bc43..f968d29e 100644 --- a/prefix-sharing/prefix_sharing/integrations/verl_mcore.py +++ b/prefix-sharing/prefix_sharing/integrations/verl_mcore.py @@ -22,7 +22,7 @@ import importlib from contextlib import contextmanager from dataclasses import dataclass -from typing import Any, Iterator, Mapping, Sequence +from typing import Any, Iterator, Mapping from prefix_sharing.backends.factory import get_backend_instance from prefix_sharing.backends.packed_layout import PackedBatchLayout @@ -34,7 +34,6 @@ from prefix_sharing.integrations.parallel_info import MegatronParallelInfo from prefix_sharing.integrations.parallel_info import get_megatron_parallel_info from prefix_sharing.integrations.patch_manager import PatchHandle -from prefix_sharing.utils import ensure_global_packed_token_lengths @dataclass(frozen=True) @@ -118,26 +117,25 @@ def build_prefix_sharing_micro_batch_verl070( # --- Path 1: prefix sharing disabled by config --- if not config.enable_prefix_sharing: - print(f"[PS][prepare] PATH 1: prefix sharing disabled (config.enable_prefix_sharing=False), returning (batch, None)") + print("[PS][prepare] PATH 1: prefix sharing disabled (config.enable_prefix_sharing=False), returning (batch, None)") return batch, None - print(f"[PS][prepare] config.enable_prefix_sharing=True, validating config...") + print("[PS][prepare] config.enable_prefix_sharing=True, validating config...") config.validate(model_config=model_config, integrate_mode="verl_megatron_actor") - print(f"[PS][prepare] config.validate() returned OK") + print("[PS][prepare] config.validate() returned OK") # --- Path 2: missing use_remove_padding --- - print(f"[PS][prepare] checking megatron.use_remove_padding...") + print("[PS][prepare] checking megatron.use_remove_padding...") if not _read_actor_bool(actor_config, "megatron.use_remove_padding", False): - print(f"[PS][prepare] PATH 2: megatron.use_remove_padding=False, raising RuntimeError") + print("[PS][prepare] PATH 2: megatron.use_remove_padding=False, raising RuntimeError") raise RuntimeError("prefix sharing phase 1 requires verl megatron.use_remove_padding=True") # --- Path 3: multi_modal check --- - print(f"[PS][prepare] use_remove_padding=True, about to batch.get(multi_modal_inputs)...") + print("[PS][prepare] use_remove_padding=True, about to batch.get(multi_modal_inputs)...") multi_modal_inputs = batch.get("multi_modal_inputs") if multi_modal_inputs is not None: # tensorclass 无法遍历（触发 CUDA 同步），改用底层 td 检查字段数 - import inspect is_tensorclass = hasattr(multi_modal_inputs, 'batch_size') print(f"[PS][prepare] multi_modal_inputs type: tensorclass={is_tensorclass}, type={type(multi_modal_inputs).__name__}") if is_tensorclass: @@ -148,9 +146,9 @@ def build_prefix_sharing_micro_batch_verl070( else: has_mm = any(mmi is not None and len(mmi.keys()) > 0 for mmi in multi_modal_inputs) if has_mm: - print(f"[PS][prepare] PATH 3: multi_modal_inputs has content, raising RuntimeError") + print("[PS][prepare] PATH 3: multi_modal_inputs has content, raising RuntimeError") raise RuntimeError("prefix sharing phase 1 supports only text-only actor micro-batches") - print(f"[PS][prepare] multi_modal check PASSED (no real multi-modal content)") + print("[PS][prepare] multi_modal check PASSED (no real multi-modal content)") # --- Read tensors --- attention_mask = batch["attention_mask"].to(bool) @@ -160,7 +158,7 @@ def build_prefix_sharing_micro_batch_verl070( # --- Path 4: wrong tensor dims --- if attention_mask.dim() != 2 or input_ids.dim() != 2 or position_ids.dim() != 2: - print(f"[PS][prepare] PATH 4: non-2D tensors detected, raising RuntimeError") + print("[PS][prepare] PATH 4: non-2D tensors detected, raising RuntimeError") raise RuntimeError("prefix sharing phase 1 expects 2D input_ids/attention_mask/position_ids") # --- Planning --- @@ -178,11 +176,11 @@ def build_prefix_sharing_micro_batch_verl070( # --- Path 5: no sharing found --- if not prefix_sharing_plan.has_sharing: - print(f"[PS][prepare] PATH 5: no sharing detected, returning (batch, None)") + print("[PS][prepare] PATH 5: no sharing detected, returning (batch, None)") return batch, None # --- Path 6: sharing found, trim the original micro-batch --- - print(f"[PS][prepare] PATH 6: sharing detected, preparing trimmed batch...") + print("[PS][prepare] PATH 6: sharing detected, preparing trimmed batch...") trimmed_micro_batch = _clone_batch(batch) new_attention_mask = attention_mask.clone() new_attention_mask[:] = False @@ -387,7 +385,6 @@ def restore_via_2d_unfold_verl080( Returns: ``output``（``log_probs``/``entropy`` 被替换为重组后的 NestedTensor）。 """ - import torch ctx = current_prefix_sharing_context() if ctx is None: @@ -412,8 +409,6 @@ def restore_via_2d_unfold_verl080( return output L_max = max(original_lengths) - device = log_probs_nested.values().device - # --- Step 1: 展开裁剪后 NestedTensor → 完整 2D [B, L_max] --- log_probs_2d, entropy_2d = _unfold_trimmed_nested_to_2d( log_probs_nested, diff --git a/prefix-sharing/prefix_sharing/setup/registry.py b/prefix-sharing/prefix_sharing/setup/registry.py index cefd65a9..982612ef 100644 --- a/prefix-sharing/prefix_sharing/setup/registry.py +++ b/prefix-sharing/prefix_sharing/setup/registry.py @@ -12,7 +12,7 @@ import builtins import sys from dataclasses import dataclass -from typing import Any, Callable +from typing import Callable from prefix_sharing.setup.logged_patch import LoggedPatchManager, PatchHandle, PatchRecord diff --git a/prefix-sharing/prefix_sharing/tools/cmp_diag.py b/prefix-sharing/prefix_sharing/tools/cmp_diag.py index 7feadafe..7f410db5 100644 --- a/prefix-sharing/prefix_sharing/tools/cmp_diag.py +++ b/prefix-sharing/prefix_sharing/tools/cmp_diag.py @@ -70,7 +70,6 @@ import logging import os from dataclasses import dataclass, field -from typing import Any import torch @@ -754,7 +753,7 @@ def cmp_2d(dir_on: str, dir_off: str, filename: str, name: str, def _print_header(dir_on, dir_off, dir_off2, tag, mask_file, layer): print(_SEP_DOUBLE) - print(f" Prefix-Sharing Diag Report") + print(" Prefix-Sharing Diag Report") print(f" ON : {dir_on}\n OFF: {dir_off}") if dir_off2: print(f" OFF2: {dir_off2}") diff --git a/prefix-sharing/prefix_sharing/tools/training_monitor.py b/prefix-sharing/prefix_sharing/tools/training_monitor.py index 6cbd506f..a6d2d88e 100644 --- a/prefix-sharing/prefix_sharing/tools/training_monitor.py +++ b/prefix-sharing/prefix_sharing/tools/training_monitor.py @@ -59,7 +59,7 @@ from contextlib import contextmanager from contextvars import ContextVar from dataclasses import dataclass -from typing import Any, Iterator, Literal +from typing import Iterator, Literal # --------------------------------------------------------------------------- diff --git a/prefix-sharing/pyproject.toml b/prefix-sharing/pyproject.toml index 610b70db..fa0a6900 100644 --- a/prefix-sharing/pyproject.toml +++ b/prefix-sharing/pyproject.toml @@ -2,3 +2,36 @@ testpaths = ["tests/unit_test", "tests/integrated_test", "tests/system_test"] pythonpath = ["."] addopts = "-ra" + +# ── ruff: gate on real bugs only (pyflakes F + syntax E9). ── +# Conservative on first introduction (codebase has never been linted; full rule +# set shows ~2700 findings). Broaden the selection as cleanup lands. +# Run: `ruff check prefix_sharing` / `ruff check --select F,E9 prefix_sharing` +[tool.ruff] +line-length = 120 +target-version = "py39" +src = ["prefix_sharing"] + +[tool.ruff.lint] +select = ["F", "E9"] + +[tool.coverage.run] +source = ["prefix_sharing"] +omit = [ + # CLI / diagnostic scripts — executed as standalone tools, not imported by tests. + "prefix_sharing/tools/*", + # verl080 monkey-patches — only activate under a real verl 0.8.0 + mcore runtime. + "prefix_sharing/setup/patches/*", + # vendor reference backends without a CPU implementation. + "prefix_sharing/backends/cann_ref.py", + "prefix_sharing/backends/cuda_ref.py", +] + +[tool.coverage.report] +show_missing = true +skip_covered = false +exclude_lines = [ + "pragma: no cover", + "raise NotImplementedError", + "if __name__ == .__main__.:", +] diff --git a/prefix-sharing/requirements-ci.txt b/prefix-sharing/requirements-ci.txt new file mode 100644 index 00000000..2a54a542 --- /dev/null +++ b/prefix-sharing/requirements-ci.txt @@ -0,0 +1,6 @@ +# CI runtime deps (installed on top of torch CPU in the workflow). +# Pinned loosely to get recent bug fixes; bump as needed. +ruff>=0.6 +coverage>=7.4 +pytest>=8.0 +numpy>=1.21