Skip to content

fix(runtime): address #506 inference review feedback - #592

Merged
pengcheng888 merged 1 commit into
refactor/adopt-modern-infini-stackfrom
fix/inference-review-feedback
Sep 22, 2026
Merged

pengcheng888 merged 1 commit into
refactor/adopt-modern-infini-stackfrom
fix/inference-review-feedback

Conversation

@voltjia

@voltjia voltjia commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Review fixes for #506, based on f21b8f9c. This PR targets refactor/adopt-modern-infini-stack.

  • Make graph-plan destruction exception-safe; prevent copied Memory/LRU ownership; keep active operator caches stable during device-vector growth.
  • Serialize trace-ring snapshots and writes, fix attention sequence-axis detection, and restore constant-time allocator free-list removal without losing graph pins or live-owner protection.
  • Remove the duplicated model allowlist, restore quantization configuration parsing, and remove model-specific short-decode/fixed-prefill graph profiles. Static attention executes eagerly rather than routing through paged kernels.
  • Forward exact per-request prefill maxima, validate metadata, and derive wrapper output head dimension from V.
  • Repair Python API inconsistencies (RoPE exports, addr parameters, stale dtype references and invalid unregistered bindings), restore T-Head device plumbing, preflight linked providers, add Moore stack-builder support, and avoid direct Linux Python-extension linkage to libpython.
  • Remove the obsolete session-planning document and add reproducible native regressions under test/native.

Motivation

Addresses the review threads on #506 and the associated review register. The reported failures include double-free/use-after-free risks, constructor-unwind crashes, a trace data race, incorrect attention metadata and undeclared deployment dependencies.

Configuration parsing is distinct from provider availability: missing quantized execution paths and dense fused MoE still report unsupported execution. No new smoke cases were added.

Type of Change

  • fix
  • build
  • Documentation and focused regression coverage accompanying the fixes

Test Results of Involved Models on Supported Platforms (Please attach screenshots)

Validated on NVIDIA A100, Python 3.12, in a fresh InfiniLM build directory against the existing matched stack (InfiniRT 0cdbb169, InfiniOps f890afb4, InfiniCCL e8ccc0cb). Results below are from command logs, not screenshots.

Validation Result
infinicore_runtime, _infinicore, _infinilm, wheel build PASS
Stack-builder unit tests 25 PASS
Static contracts 101 PASS
Native analyzer/runtime/config/paged-metadata suites 4 PASS under ASan/UBSan
Trace concurrency with 100k writes, wraparound and clear TSan PASS
Existing NVIDIA runtime/stream/packed SwiGLU tests 8 PASS
Qwen3-0.6B, FlashAttention eager, TP=1 PASS
Qwen3-0.6B, FlashAttention graph, TP=2 PASS; generic segmented graph captured on both ranks
Qwen3-0.6B, paged attention, batch=2 PASS
Qwen3-0.6B, static attention with graph requested PASS through eager execution
Wheel imported outside source tree; both extension ELF dependencies inspected PASS; no direct libpython dependency

All four inference variants generated matching 16-token greedy response text for the same prompt. This is a short inference regression, not an accuracy or throughput benchmark. Other accelerator hardware, Python 3.13 and multi-stage PP were not exercised in this revision.

The image's standalone Torch import fails with an empty LD_LIBRARY_PATH because of its HPCX/UCX ucs_config_doc_nop mismatch. The successful source-external wheel import used only the image's HPCX directories, with no Python-library path. CUDA's ASan virtual-address conflict required ASAN_OPTIONS=protect_shadow_gap=0; production behavior is unchanged.

Benchmark / Performance Impact

No throughput improvement claim. Free-list removal changes from scanning/erase to indexed swap/pop. Static graph emulation, fixed Baichuan prefill and model-name-specific short-decode acceleration are removed; affected requests use generic decode graphs or eager execution.

Notes for Reviewers

  • Graph plans are only owned after all dispatcher lookups succeed. Device-cache vector growth never copies a cache.
  • Unregistered binding headers do not imply public Python support: five invalid headers were removed; remaining legacy headers were not bulk-registered.
  • Boolean dtype requires an InfiniRT API addition; this PR does not alias UInt8 as bool.
  • Quantization metadata parsing is restored, but quantized inference and dense fused MoE are not claimed as working.
  • The paged-prefill wrapper now follows V's head dimension; the existing GPU provider still constrains Q/K/V head layouts.
  • T-Head wiring and Moore build options have code/config coverage; hardware execution on those platforms remains unverified.

CI / ChatOps

Local and remote validation is recorded above. NVIDIA CI was dispatched on this branch with InfiniCore main (26f7382d, the merged component manifest). Its format and matrix-generation jobs passed; the NVIDIA build/package job is queued. PR format and Ruff checks also passed. CI status is separate from the completed NVIDIA runtime evidence.

Checklist

  • Conventional title, branch name and single reviewable commit.
  • Based directly on refactor(runtime)!: adopt the modern Infini stack #506's head branch; no unrelated main-branch merge.
  • Changes correspond to the review feedback, with limitations stated.
  • Repository formatter, static tests, unit tests and native regressions passed.
  • Fresh native build and wheel packaging passed.
  • Single-request and TP inference checked; no new smoke tests.
  • README, CONTRIBUTING and native test instructions updated.
  • No secrets or machine-specific paths committed.
  • Other accelerator hardware and Python 3.13 deployment validation.
  • Offline benchmark, full model sanity matrix and service load tests (not run; this PR makes no performance/service-validation claim).
  • Repository CI completion.

@pengcheng888
pengcheng888 self-requested a review September 22, 2026 07:52

@pengcheng888 pengcheng888 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

一、已解决的一些问题:
(1) csrc/config/config_factory.cpp中对模型的枚举已经删除。
(2) 代码中看起来有了CompressedTensors,AWQ,GPTQ,MXFP4量化代码。也有cache int8量化。

二、其他改动:
(1) 其他可有可无的改动。例如移除中文注释,看起来等价的代码重新组织等。

三、疑问:
(1) csrc/infinicore/src/pybind11/ops/中有多个算子接口文件的移除,这是为啥?

四、仍有的问题:
(1) csrc/engine/compiler/paged_compiler.cpp/static_batching_compiler.cpp ,有挺多改动,未细看,建议恢复。
(2) kv cache的两种形式的shape排布,还没修改回来。
(3) 三个attn计算后端的算子仍没有完全对上。

仍存在的问题会在开发分支上提pr处理。

@pengcheng888
pengcheng888 merged commit 1d06427 into refactor/adopt-modern-infini-stack Sep 22, 2026
10 of 16 checks passed
@voltjia

voltjia commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

补充说明这条总评里的接口删除问题:#592 删除的是 5 个未注册且无效的 Python binding header:all、equal、logical_and、logical_not 引用了不存在的 public header;scaled_mm_i8 的函数声明/参数不匹配。它们不参与当前 Python 模块注册,删除没有减少已可调用的 Python API,也没有删除对应的有效 C++ 量化接口。其余未注册 legacy binding 没有批量删除或注册。

csrc/layers/quantization 的 Marlin/QY 类接口是另一件事。按后续意见,#594 已恢复 AWQMarlin、GPTQMarlin、GPTQ_QY 的类/枚举/聚合头与元数据接口,实际执行处明确报 unsupported;配置或接口存在仍不代表量化推理可用。

compiler 改动中删除的是固定 Baichuan prefill、模型名专用 short-decode 和 static 借用 paged graph,保留通用 decode graph 与输入/生命周期保护。attention 后端后续以已合入的 #593 为准;#594 只修正 CLI/benchmark 仍默认选择已移除后端的入口遗漏,并恢复 ali 兼容别名,没有回退 #593。

后续修正及验证:#594

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants