You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
🔴 Brittle version assertion — Line 140: torch.__version__ == '2.7.1+cpu' will fail spuriously on macOS or musl runners where the local tag differs (e.g., 2.7.1+cpu.macosx or 2.7.1+cpu.aarch64). Suggestion: assert startswith('2.7.1') and check torch.version.cuda is None separately.
🔴 Install order fragility — pip install -e ".[all]" (line 126) may already install a torch (possibly CUDA build); then you reinstall torch==2.7.1+cpu which downgrades in place. pip check catches conflicts, but mixing an [all] extra that expects CUDA torch with a CPU torch can leave torch's +cu submodules dangling. Suggestion: install CPU torch first, or pass --force-reinstall --no-deps on the torch line, and add a guard like python -c "import torch; assert not torch.cuda.is_available()".
🟡 Single-thread env vars leak into benchmark semantics — Lines 205–207 set OMP/OPENBLAS/MKL_NUM_THREADS=1 for the whole benchmark job. This forces serial execution and will produce timings that don't reflect real-world throughput. If the goal is determinism (hash seed is already set), consider scoping these only to the specific subtests that flake under parallelism, and document the choice in a comment so future readers don't "fix" it.
🟡 requirements/qwen3-small-probe.txt not validated — You pip install -r it but never verify it's tracked, non-empty, or pinned. If the file is added as a path filter exclusion, or is accidentally empty, this step silently no-ops. Suggestion: test -s requirements/qwen3-small-probe.txt at the top, and prefer fully-pinned requirements over transitive resolution for reproducible CI.
🟡 No retry/timeout on PyTorch wheel download — download.pytorch.org/whl/cpu is occasionally flaky and the wheel is large; a single transient failure fails the whole job. Consider --retries 3 --timeout 60 on that install line.
💭 Assertion message doesn't indicate which condition failed — Split into two asserts, or include torch.version.cuda in the failure message so debugging doesn't require rerunning.
💭 Comment says "instead of skipping those tests" but doesn't link to the prior skip — If there's a previous PR/commit that removed skips, a Refs: #1234 would make the history discoverable.
📁 .github/workflows/llm-deploy.yml
🟡 Summary job runs unconditionally on all events — w3-execution-summary has if: always() and needs: [w3-validation], so it runs on every PR and every nightly schedule, just to write "NOT RUN." That's an extra runner slot per PR for no value. Consider adding if: always() && github.event_name == 'workflow_dispatch' so it only surfaces when someone actually used manual dispatch.
🟡 Inconsistent scope naming — workflow_dispatch.inputs.run_w3_validation.description says "seven-case IR validation" but the summary text says "full 28-layer IR numeric." Pick one terminology so reviewers don't wonder if two different things were meant.
💭 Heredoc indentation — The Python block in the heredoc uses 4-space indent consistently, which is correct for <<'PY'. Just noting that if anyone later converts this to an inline python -c or extracts it to a script, the heredoc structure won't survive. Consider extracting to scripts/w3_summary.py for testability, since it has branching logic with 4 cases.
📁 .github/workflows/w3-full-numeric.yml
Code Review
🟡 Inconsistent action pinning — actions/checkout@v4 and actions/upload-artifact@v4 use floating tags while actions/setup-python is pinned to a SHA. A tag update to v4 could silently change CI behavior. Pin all three to commit SHAs.
🟡 Contradictory input descriptions — workflow_dispatch says "subset returns PARTIAL if numerically correct" but workflow_call says "partial coverage never passes acceptance." Downstream consumers can't tell whether partial results are valid. Align the descriptions with actual acceptance semantics.
🟡 Duplicated artifact path globs — Both upload steps share ~8 path patterns. If retention policy changes (e.g. add *.schema), two blocks must be updated in lockstep. Consider a shared include or at minimum a comment linking them.
🟡 workflow_call input type mismatch — workflow_dispatch.case is type: choice (enum-enforced) but workflow_call.case is type: string. A caller passing a typo like "short_17 " (trailing space) reaches the shell validation instead of failing at YAML parse time. Consider type: choice on both, or add a comment explaining why string is intentional.
💭 pip install -e . in CI — Editable installs can pull in untracked local files and are harder to reproduce. For CI, a regular pip install . (or pip wheel) gives a more hermetic result and avoids silent changes from workspace state.
💭 SELECTED_CASE default logic is split — The env block uses ${{ inputs.case || 'all' }}, the validation step uses ${SELECTED_CASE:-all}, and the probe step re-reads ${SELECTED_CASE:-all}. Three independent defaults for the same variable. Collapse to one source of truth (the env assignment) and reference $SELECTED_CASE everywhere else.
Summary: No blockers. Pinning, description alignment, and the triple-default cleanup are the highest-leverage changes before merging.
📁 .github/workflows/w3-nightly.yml
🔴 Bug: workflow_call 触发被门控条件静默吞掉 — Line 4, 15, 18, 22
💭 nit — 表格行名 "Complete seven-case Qwen3 IR numeric" 与实际 case 数耦合在 workflow 里,测试矩阵扩到八个 case 时需要回来改文案;可接受,提醒一下。
📁 .github/workflows/w3-preparation.yml
Review: .github/workflows/w3-preparation.yml
🟡 Inconsistent action pinning — setup-python is pinned by full SHA (a26af69be9...), but actions/checkout@v4 and actions/upload-artifact@v4 are pinned only by major version. This means checkout and upload-artifact can silently break or be compromised by upstream updates.
Suggestion: Pin all third-party actions by full SHA. Consider adding a Dependabot or Renovate config to keep SHAs current.
🟡 No dependency caching — Every run reinstalls torch (≈200MB+), qemu, and the full Python environment from scratch. With a 60-min timeout and workflow_dispatch, this wastes significant wall-clock time.
Suggestion: Add actions/cache for ~/.cache/pip and ~/.cache/huggingface (key on commit hash + dependency file hashes).
🟡 cancel-in-progress: false — With a 60-min timeout, re-dispatching or a new workflow_call will queue a second full run alongside the first. On a self-hosted or constrained runner, this could double resource usage for no benefit.
Suggestion: Set cancel-in-progress: true — the latest dispatch should supersede the older one.
💭 Fragile SCRATCHV_CC quoting — Line 51 embeds a nested python -c inside a $(...) inside a "..."-quoted echo. It works, but the three layers of quoting make it hard to audit and easy to break on edit.
Suggestion: Use a heredoc or a simple printf assignment:
💭 No retry on snapshot_download — HuggingFace downloads from a shared CI network can intermittently fail. snapshot_download defaults to max_workers=8 but no retry policy is applied.
Suggestion: Wrap with tenacity or set retries=3 if available.
💭 apt-get install without pinned package version — qemu-system-misc version drifts between Ubuntu releases. Not a blocker for a preparation workflow, but worth a comment noting the base image contract.
📁 LICENSES/Eigen-MPL-2.0.txt
🔴 License correctness — verify text is canonical — This is the MPL 2.0 from Eigen's repo. The text should be byte-for-byte identical to https://www.mozilla.org/MPL/2.0/ (or the SPDX MPL-2.0 entry). A drifted/reformatted copy can create ambiguity about which terms actually govern. Suggestion: download from the canonical source and diff before committing; add a SPDX-License-Identifier: MPL-2.0 header line at top if not already present elsewhere, so tooling (e.g. reuse, fossid, licensee) can detect it.
🟡 Directory/name convention — SPDX expects lowercase — SPDX License List convention is LICENSES/<SPDX-ID>.txt where the ID preserves its original case, but the directory is usually LICENSES/ (OK) and file basename is the SPDX ID with .txt. Eigen-MPL-2.0.txt is not a valid SPDX ID — MPL-2.0 is. The Eigen- prefix is non-standard and will confuse SPDX/reuse tooling that maps filename → identifier. Suggestion: rename to LICENSES/MPL-2.0.txt and record the "Eigen uses MPL-2.0" mapping in a THIRD_PARTY_NOTICES or NOTICE file instead.
🟡 Missing NOTICE / attribution file — Adding a dependency's license under LICENSES/ is good practice (REUSE compliance), but without a corresponding THIRD_PARTY_NOTICES or a LICENSES.md / NOTICE entry mapping "Eigen → MPL-2.0, SPDX ref MPL-2.0, upstream https://eigen.tuxfamily.org", auditors can't tell why this file is here. Suggestion: add an entry in your notices file with project name, version, SPDX ID, and upstream URL.
🟡 Verify Eigen version compatibility — Eigen was LGPL//MPL-2.0 dual-licensed before 3.3, and is MPL-2.0 only in 3.3+. If this project pins or transitive-depends on Eigen, confirm the actual resolved version matches the license you're shipping. Shipping MPL-2.0 for a 3.2.x (LGPL) pin would be misleading.
💭 End-of-file newline / trailing whitespace — Confirm the file ends with a single \n and no trailing spaces; many tools (including SPDX) complain about trailing whitespace in license files.
💭 Consider also shipping a LICENSES/README.md — A one-paragraph note explaining that files under LICENSES/ are verbatim copies of upstream third-party licenses (with source URL) helps future contributors not "tidy up" the text and accidentally alter terms.
Overall the change itself (adding a third-party license verbatim) is the right move; the issues above are about making it tooling-detectable and audit-friendly.
📁 THIRD_PARTY_NOTICES.txt
🔴 Bug: Incorrect license attribution for ONNX Runtime — Line 12: "under MIT" is wrong. ONNX Runtime (and MLAS within it) is licensed under Apache-2.0, not MIT. MIT and Apache-2.0 have different attribution terms (patent grant, NOTICE file requirements). Publishing this in a NOTICE file could be a legal/compliance issue.
Suggestion: Verify against onnxruntime/LICENSE for tag v1.22.1. If MLAS has its own nested license (some mlas/*.c headers are BSD-3-Clause in older releases), cite the actual one.
🔴 Bug: Eigen license may be incomplete — Line 7: Eigen core is MPL-2.0, but the Eigen tree also contains BSD-2-Clause files (test/support/*, doc/doxygen/*, cmake/*). If the derived code touches those, the notice is incomplete.
Suggestion: Confirm the derived code only pulls from Eigen/src/*. If yes, add an explicit scope statement like "derived from Eigen Eigen/src/... only (MPL-2.0); no BSD-licensed Eigen files are included."
🟡 Missing SPDX identifiers — Best practice is machine-readable identifiers.
Suggestion: Rewrite as License: MPL-2.0 (SPDX) / License: Apache-2.0 (SPDX) for automated license scanners (FOSSA, LicenseFinder, etc.) to pick up correctly.
🟡 Version provenance asymmetry — Eigen is pinned to a commit hash (good), ONNX Runtime is pinned to a tag (good), but neither has a URL to the upstream source. Reproducibility suffers.
Suggestion: Add source URLs, e.g. https://gitlab.com/libeigen/eigen/-/commit/1d8b82b... and https://github.com/microsoft/onnxruntime/releases/tag/v1.22.1.
🟡 Header wording is ambiguous — Line 2: "Package metadata lists both applicable licenses; it does not replace these file-specific notices." The reader cannot tell what package metadata (PKG-INFO? Cargo.toml? package.json?) or which two licenses.
Suggestion: Name the file and the licenses explicitly, e.g. "The project's PYPROJECT.tomllicense field lists MIT and MPL-2.0; this file documents file-specific obligations."
🟡 Missing "modified" disclosure — Both files are described as "Derived from" / "adapted from", but the license texts don't require you to state what was modified. For MPL-2.0 specifically, marking modifications is a practical obligation for downstream users.
Suggestion: Add a one-line pointer, e.g. "Local modifications are limited to Python porting, type coercion, and removal of Eigen headers."
💭 Nit: Path prefix style — Line 4–5 and 10–11 use bare paths (scratchv/verification/...). If the project has a pyproject.toml or setup.py under the repo root, consider whether paths should be relative to repo root vs. package root, matching how the rest of the repo references them.
💭 Nit: Consider a separate NOTICE file convention — Apache-2.0 (if applicable) conventionally uses a top-level NOTICE file rather than THIRD_PARTY_NOTICES.txt. Since one of the dependencies should be Apache-2.0 (see 🔴 #1), you may want to rename or add a NOTICE file to match convention.
💭 Nit: No date or maintainer contact — NOTICE files that last years benefit from a "Last updated: YYYY-MM-DD" line and a contact address for license questions.
📁 benchmarks/bench_onnx_operators.py
🟡 Suggestion: .exists() doesn't guarantee a valid repo — A stale/corrupt .git directory would pass the check but still fail (or produce nonsense) on the subsequent git call, defeating the guard's purpose. Consider either catching subprocess.CalledProcessError from git rev-parse --git-dir as the canonical validation, or at minimum appending the git rev-parse --is-inside-work-tree check into the same subprocess call to get a fast, atomic validation.
📁 docs/llm-deploy-v1.0/W1/W1-报告可靠性修复.md
🔴 Historical link points to wrong commit — The "固定历史原文" link points to 77c3aa8..., but the fix merged as faab6a8.... If 77c3aa8... is the W3 integration branch (which already contains the fix), the "original pre-fix text" won't be recoverable from that SHA. The link should point to the commit before PR #93 was applied (the PR base or the original W1 report commit).
🟡 .venv-linux referenced but never created — source .venv-linux/bin/activate is added to the reproduction block, but there's no instruction for how to create this venv. A reproducer following these steps will fail immediately (good, thanks to set -e), but won't know why. Either add setup commands or link to the venv creation guide in LINUX_REPRODUCTION.md.
🟡 Detailed metrics silently dropped — The old text included "262 passed / 0 skipped", "7 组输入、29 个检查点及不变性", and wall-clock timings. These are removed in favor of the Linux W3 numbers. The new text acknowledges the Linux numbers are from a different run, but the Windows-specific test counts (which document what the original verification actually covered) are now only accessible via the historical link. Consider adding a one-line cross-reference like "完整 Windows 原始指标见上方历史链接" so readers know the granular data still exists.
💭 Action run URL — actions/runs/37303495582 is a 14-digit ID. Standard GitHub Actions run IDs are typically 12-15 digits, so this is plausible but worth verifying it resolves correctly before merge — broken evidence links undermine the reliability report's credibility.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
完整 Qwen3 已能解析,但仍需要真实 28 层 IR 与 ORT 的数值验证及误差定位。本 PR 补齐从六层模型、真实权重子图、小 Attention QEMU 到完整 IR 的验证与复现入口。
W2 基础已由 #106 合入,#94 已关闭;本分支同步主干
24895c7。完整 RV64/QEMU 前向由 #104 提供。修改内容
本次同步与修正(2026-10-13)
all目前不执行乘加融合;当前执行环境与早期导出环境分别记录。已有完整验证
完整 W3 Actions Summary,执行提交
210baa5fbd87844e61e5dfb273b49748a978b708(2026-10-11):上述结果属于记录的提交、reference-v4/avx512 和执行环境,不能改标为当前 HEAD 的完整模型结果。
复现入口与验收边界
完整入口为
probes/w3_qwen3_full/run.py --fp32-mode reference;正式完整验收必须运行全部七组。手动工作流使用run_w3_validation=true。报告与原始数组分别保存,受 artifact 保留期限制。默认解释器 native 模式不变;通过结论不覆盖任意 CPU/BLAS、优化等级或 ORT 构建。W3 的完整模型结果是 Host IR;QEMU 证据限于小 Attention。既有 PyTorch/ORT 导出差异单独保留,不因 IR/ORT 通过而消失。
仍待团队确认 reference 契约、阶段前置、E1/E2/E5 独立复现和真实 schedule Nightly;
w3_exit_accepted保持 false。