Conversation
LauraGPT
left a comment
There was a problem hiding this comment.
Thanks for contributing the Ascend guide. I compared the exact documented seg_pooling function at a082a27 against CAMLayer.seg_pooling from base486b4b7 using real torch2.10.0+cpu, without loading model weights or changing either function. The proposed replacement is not numerically equivalent for incomplete final segments.
For torch.ones(1,1,150), seg_len=100, avg mode: the original returns 1 throughout, but the replacement returns 0.5 on the final50 frames. At T=101 it returns0.01 instead of1 on the final frame. For all-negative input in max mode, the padded tail returns0 instead of-1. Across lengths1/50/100/101/150/200/201 and both modes, the four full-segment controls match and all ten incomplete-segment cases differ; shapes and caller inputs are preserved.
These are deterministic CPU checks of pooling semantics, not independent verification of the reported NPU setup, NPU operator support, speaker accuracy or full pipeline. Please correct the final-segment reduction and add executable equivalence regressions before recommending this as an equivalent CAM++ patch.
| pad = (-T) % seg_len # 等价 ceil_mode | ||
| xp = F.pad(x, (0, pad), value=0.0) if pad else x | ||
| xg = xp.reshape(B, C, (T + pad) // seg_len, seg_len) | ||
| seg = xg.mean(dim=-1) if stype == "avg" else xg.max(dim=-1).values |
There was a problem hiding this comment.
[P2] Preserve the original reduction on incomplete tail segments. Explicit zero padding becomes part of xg.mean(), whereas the original ceil-mode pooling averages only the real tail elements; for150 all-one frames this changes the last50 outputs from1 to0.5. CAMLayer.forward directly adds this result into the attention context, so this changes model computation rather than merely working around an unsupported operator. Zero padding also changes max pooling for a negative-only tail. Reduce only valid elements (and preserve max/invalid-stype behavior), with regressions for short/non-multiple lengths and negative max inputs; verify any replacement on NPU separately.
|
Thanks for the careful review — you're right. The explicit zero padding polluted the incomplete tail segment: it got included in the mean (0.5 instead of 1.0 for the T=150 all-ones case) and corrupted max pooling on all-negative tails (0 instead of -1). I've replaced the padding approach with a slice-based implementation (commit ead45ae): full segments are pooled normally; the incomplete tail is reduced over its REAL frames only — no padding at all, so max/invalid-avg semantics are preserved. Equivalence regression (CPU, torch 2.10.0, vs avg_pool1d/max_pool1d with ceil_mode=True): lengths {50, 99, 100, 101, 110, 150, 199, 200, 201, 250, 1000} x {avg, max} x {all-ones, all-negative, randn} — 66 cases, all pass torch.allclose(rtol=1e-4, atol=1e-6). Your three examples now match the original exactly (1.0 / 1.0 / -1.0). Residual differences are float32 accumulation-order noise only (max ~1.2e-7). You're also right that this is a CPU semantics check rather than independent NPU-side verification; the doc now labels the regression as such. Updated file: docs/ascend_npu.md @ ead45ae. |
LauraGPT
left a comment
There was a problem hiding this comment.
The incomplete-tail reduction is now fixed in my bounded CPU check of ead45ae. With the missing torch global supplied to isolate the mathematics, I compared the exact revised function against base CAMLayer.seg_pooling using torch2.10.0+cpu, B=2,C=3, your11 lengths x avg/max x ones/negative/random(seed0):66/66 torch.allclose checks pass(rtol1e-4,atol1e-6), largest absolute difference5.96e-8. Shapes and caller inputs are unchanged. The previous zero-padding discrepancy should no longer be treated as unresolved on this head.
One runnable-example fix remains: add import torch to the code block. Neither its existing 'from ... import components as cp' nor 'import torch.nn.functional as F' binds the name torch, so a fresh namespace fails at torch.cat with T=150: NameError: name 'torch' is not defined. Also, the revised conditional still treats an unknown stype as max, whereas the original raises ValueError; please retain that small validation if claiming the same method contract.
This verifies CPU reductions only, not the new slice/cat implementation on Ascend. Please distinguish the originally reported hardware run from any rerun with this revised snippet, and retain the CPU-only label for the numerical regression. No model-weight inference, NPU compatibility or accuracy/performance acceptance is claimed here.
| tail_x = x[:, :, n_full * seg_len:] | ||
| segs.append(tail_x.mean(dim=-1, keepdim=True) if stype == "avg" | ||
| else tail_x.max(dim=-1, keepdim=True).values) | ||
| seg = torch.cat(segs, dim=-1) if len(segs) > 1 else segs[0] # [B, C, nseg] |
There was a problem hiding this comment.
[P2] Import torch before using the revised snippet. The shown imports bind cp and F, not torch. When the input has both a full segment and an incomplete tail (for example T=150 with seg_len=100), this branch raises NameError instead of computing the pooling result. Add import torch to the code block and check it in a fresh namespace.
|
Both remaining points are fixed in 27fc1ed:
Understood on scope: the CPU-only label stays on the numerical regression, and the doc's scope note distinguishes the originally reported 310P3 hardware run (old patch) from any rerun with this revised snippet. Updated file: docs/ascend_npu.md @ 27fc1ed. |
LauraGPT
left a comment
There was a problem hiding this comment.
Verified the follow-up at27fc1ed. I executed the complete shown CAM++ code block in a fresh namespace using the installed CPU runtime(torch2.10.0+cpu), including the actual components import and method assignment. The T150 all-one avg and all-negative max controls pass, and unknown stype now raises the original ValueError. AST comparison confirms that the reduction body is unchanged from ead45ae apart from the added validation, so I did not rerun the already completed66-case numerical matrix.
The earlier tail bias, missing torch import and invalid-stype findings are addressed within these checks. The revised CPU-only scope note is present. I have not independently run this implementation on Ascend, loaded model weights, or assessed speaker accuracy; the reported initial hardware run is separate evidence. This is a verification update, not a merge or hardware-compatibility approval.
Summary
新增《在昇腾 NPU(Ascend)上运行 FunASR》文档(docs/ascend_npu.md),内容涵盖:
Type of change
Validation
在 Atlas 300I Pro(Ascend 310P3)、CANN 9.1.0-beta.1、torch 2.10.0+cpu、torch_npu 2.10.0、funasr 1.3.14 环境实测:
仅文档变更,无任何代码改动。文档挂载位置可按仓库规范调整。