test(a5/tmr): port a2a3-only tensormap_and_ringbuffer scene/L3 tests to a5#1450
test(a5/tmr): port a2a3-only tensormap_and_ringbuffer scene/L3 tests to a5#1450yanghaoran29 wants to merge 2 commits into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds multiple TensorMap and ringbuffer scene implementations: tiled matmul/add, batched and SPMD paged attention, fan-in and dispatch regression scenarios, plus runtime registration, dependency, host-buffer, and prewarm tests. ChangesAlternating matmul and add
Batched paged attention
Four-dimensional paged attention
SPMD TPUSH/TPOP attention
Runtime lifecycle and L3 behavior
Fan-in and dispatch regression coverage
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
tests/st/a5/tensormap_and_ringbuffer/multi_round_paged_attention/test_multi_round_paged_attention.py (1)
30-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument that scene
CALLABLEandCASESmetadata must remain read-only.These mutable class attributes are shared across test cases; any recursive update during path resolution or validation may leak between cases. Mark them read-only/read-only-convention-compliant with
typing.ClassVaror a narrow RUF012 suppression if mutation is never intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/st/a5/tensormap_and_ringbuffer/multi_round_paged_attention/test_multi_round_paged_attention.py` around lines 30 - 66, Mark the shared class attributes CALLABLE and CASES as read-only metadata by annotating them with typing.ClassVar, or apply a narrow RUF012 suppression if they must remain mutable by convention. Update both affected declarations in tests/st/a5/tensormap_and_ringbuffer/multi_round_paged_attention/test_multi_round_paged_attention.py:30-66 and :68-134, without changing their contents or runtime behavior.Source: Linters/SAST tools
tests/st/a5/tensormap_and_ringbuffer/fanin_lookup_perf/test_fanin_lookup_perf.py (1)
30-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate shared class configuration with
ClassVar.Ruff RUF012 flags both mutable class attributes. These are intentionally class-level scene metadata, so annotate them as
ClassVarto make the ownership explicit and clear the warning without changing runtime behavior.Proposed fix
import ctypes +from typing import ClassVar ... - CALLABLE = { + CALLABLE: ClassVar = { ... } - CASES = [ + CASES: ClassVar = [ ... ]Also applies to: 48-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/st/a5/tensormap_and_ringbuffer/fanin_lookup_perf/test_fanin_lookup_perf.py` around lines 30 - 46, Annotate the mutable class-level scene metadata containing CALLABLE and the corresponding configuration at the other flagged location with typing.ClassVar. Preserve their existing values and class-level behavior while clearing Ruff RUF012.Source: Linters/SAST tools
tests/st/a5/tensormap_and_ringbuffer/spmd_batch_dispatch_oob/test_spmd_batch_dispatch_oob.py (1)
38-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate mutable class configuration as
ClassVar.
CALLABLEandCASESare intentionally class-level configuration, but their mutable types trigger Ruff RUF012. AddClassVarannotations to clear the warning without moving them to instance state.Proposed fix
+from typing import ClassVar + ... - CALLABLE = { + CALLABLE: ClassVar[dict] = { ... - CASES = [ + CASES: ClassVar[list] = [Also applies to: 66-73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/st/a5/tensormap_and_ringbuffer/spmd_batch_dispatch_oob/test_spmd_batch_dispatch_oob.py` around lines 38 - 64, Annotate the class-level mutable configuration attributes CALLABLE and CASES with typing.ClassVar, preserving their existing values and keeping them on the class rather than moving them to instance state.Source: Linters/SAST tools
tests/st/a5/tensormap_and_ringbuffer/spmd_paged_attention/kernels/orchestration/spmd_paged_attention_orch.cpp (1)
64-64: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueAlign the SPMD paged-attention orchestration entry with the existing AICPU entry convention. All other orchestration kernels under
tests/st/a5/tensormap_and_ringbufferexportaicpu_orchestration_entry, but the graph-basedmulti_round_paged_attentionport usesbuild_paged_attention_graph; pick one entry ABI for this test and keep the kernel export andfunction_namematching it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/st/a5/tensormap_and_ringbuffer/spmd_paged_attention/kernels/orchestration/spmd_paged_attention_orch.cpp` at line 64, Align the SPMD paged-attention entry ABI across the orchestration kernel and test configuration: choose either aicpu_orchestration_entry or build_paged_attention_graph, then make the exported function in spmd_paged_attention_orch.cpp and the function_name in test_spmd_paged_attention.py use the same symbol, matching the convention selected for this test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@tests/st/a5/tensormap_and_ringbuffer/alternating_matmul_add/kernels/orchestration/alternating_orch.cpp`:
- Around line 70-121: Validate matmul_batch and add_batch before the group-count
divisions in the orchestration setup, rejecting zero values and any
configuration where total_matmul_tasks or total_add_tasks is not evenly
divisible by its corresponding batch size. Use the existing runtime assertion or
validation mechanism, and only compute num_matmul_groups and num_add_groups
after these checks so no tasks are silently dropped.
In
`@tests/st/a5/tensormap_and_ringbuffer/batch_paged_attention/kernels/aic/aic_qk_matmul.cpp`:
- Around line 128-143: Case3 dispatches head_dim=256 but the q_tile-64 kernels
only support 128-wide data. Add matching head_dim=256 template dispatches in
aic_qk_matmul.cpp (qk_matmul_batch_impl), aic_pv_matmul.cpp (PV implementation),
and aiv_online_update.cpp (online update), preserving the existing 128 paths for
other inputs; update test_batch_paged_attention.py only if the wide-head-dim
kernels are not implemented, by removing or guarding Case3.
In
`@tests/st/a5/tensormap_and_ringbuffer/paged_attention_unroll_4dims/test_paged_attention_unroll_4dims.py`:
- Around line 98-113: Disable or remove the manual Case3 configuration in the
test definition, unless the paged-attention orchestration and its q_tile=64 QK,
PV, softmax, and update branches are updated to support head_dim=256. Do not
leave Case3 enabled while only head_dim=128 templates process the workload.
In `@tests/st/a5/tensormap_and_ringbuffer/test_l3_host_buffer_registration.py`:
- Around line 25-28: Update the documentation in the host-buffer registration
test to replace stale a2a3/a2a3sim references with a5/a5sim, including the
statement about vector_example kernels, so it accurately matches the test’s
a5sim runtime and KERNELS_BASE configuration.
---
Nitpick comments:
In
`@tests/st/a5/tensormap_and_ringbuffer/fanin_lookup_perf/test_fanin_lookup_perf.py`:
- Around line 30-46: Annotate the mutable class-level scene metadata containing
CALLABLE and the corresponding configuration at the other flagged location with
typing.ClassVar. Preserve their existing values and class-level behavior while
clearing Ruff RUF012.
In
`@tests/st/a5/tensormap_and_ringbuffer/multi_round_paged_attention/test_multi_round_paged_attention.py`:
- Around line 30-66: Mark the shared class attributes CALLABLE and CASES as
read-only metadata by annotating them with typing.ClassVar, or apply a narrow
RUF012 suppression if they must remain mutable by convention. Update both
affected declarations in
tests/st/a5/tensormap_and_ringbuffer/multi_round_paged_attention/test_multi_round_paged_attention.py:30-66
and :68-134, without changing their contents or runtime behavior.
In
`@tests/st/a5/tensormap_and_ringbuffer/spmd_batch_dispatch_oob/test_spmd_batch_dispatch_oob.py`:
- Around line 38-64: Annotate the class-level mutable configuration attributes
CALLABLE and CASES with typing.ClassVar, preserving their existing values and
keeping them on the class rather than moving them to instance state.
In
`@tests/st/a5/tensormap_and_ringbuffer/spmd_paged_attention/kernels/orchestration/spmd_paged_attention_orch.cpp`:
- Line 64: Align the SPMD paged-attention entry ABI across the orchestration
kernel and test configuration: choose either aicpu_orchestration_entry or
build_paged_attention_graph, then make the exported function in
spmd_paged_attention_orch.cpp and the function_name in
test_spmd_paged_attention.py use the same symbol, matching the convention
selected for this test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4306e68d-14ef-46f2-a446-2e70cba34302
📒 Files selected for processing (32)
tests/st/a5/tensormap_and_ringbuffer/alternating_matmul_add/kernels/aic/kernel_matmul.cpptests/st/a5/tensormap_and_ringbuffer/alternating_matmul_add/kernels/aiv/kernel_add.cpptests/st/a5/tensormap_and_ringbuffer/alternating_matmul_add/kernels/orchestration/alternating_orch.cpptests/st/a5/tensormap_and_ringbuffer/alternating_matmul_add/test_alternating_matmul_add.pytests/st/a5/tensormap_and_ringbuffer/batch_paged_attention/kernels/aic/aic_pv_matmul.cpptests/st/a5/tensormap_and_ringbuffer/batch_paged_attention/kernels/aic/aic_qk_matmul.cpptests/st/a5/tensormap_and_ringbuffer/batch_paged_attention/kernels/aiv/aiv_online_update.cpptests/st/a5/tensormap_and_ringbuffer/batch_paged_attention/kernels/aiv/aiv_softmax_prepare.cpptests/st/a5/tensormap_and_ringbuffer/batch_paged_attention/kernels/orchestration/paged_attention_orch.cpptests/st/a5/tensormap_and_ringbuffer/batch_paged_attention/test_batch_paged_attention.pytests/st/a5/tensormap_and_ringbuffer/dynamic_register/test_dynamic_register.pytests/st/a5/tensormap_and_ringbuffer/fanin_lookup_perf/kernels/aic/kernel_write_const_visible.cpptests/st/a5/tensormap_and_ringbuffer/fanin_lookup_perf/kernels/orchestration/fanin_lookup_perf_orch.cpptests/st/a5/tensormap_and_ringbuffer/fanin_lookup_perf/test_fanin_lookup_perf.pytests/st/a5/tensormap_and_ringbuffer/multi_round_paged_attention/test_multi_round_paged_attention.pytests/st/a5/tensormap_and_ringbuffer/paged_attention_unroll_4dims/kernels/aic/aic_pv_matmul.cpptests/st/a5/tensormap_and_ringbuffer/paged_attention_unroll_4dims/kernels/aic/aic_qk_matmul.cpptests/st/a5/tensormap_and_ringbuffer/paged_attention_unroll_4dims/kernels/aiv/aiv_online_update.cpptests/st/a5/tensormap_and_ringbuffer/paged_attention_unroll_4dims/kernels/aiv/aiv_softmax_prepare.cpptests/st/a5/tensormap_and_ringbuffer/paged_attention_unroll_4dims/kernels/orchestration/paged_attention_orch.cpptests/st/a5/tensormap_and_ringbuffer/paged_attention_unroll_4dims/test_paged_attention_unroll_4dims.pytests/st/a5/tensormap_and_ringbuffer/spmd_batch_dispatch_oob/kernels/aic/kernel_write.cpptests/st/a5/tensormap_and_ringbuffer/spmd_batch_dispatch_oob/kernels/aiv/kernel_write.cpptests/st/a5/tensormap_and_ringbuffer/spmd_batch_dispatch_oob/kernels/orchestration/spmd_batch_dispatch_oob_orch.cpptests/st/a5/tensormap_and_ringbuffer/spmd_batch_dispatch_oob/test_spmd_batch_dispatch_oob.pytests/st/a5/tensormap_and_ringbuffer/spmd_paged_attention/kernels/mix/paged_attention_parallel.cpptests/st/a5/tensormap_and_ringbuffer/spmd_paged_attention/kernels/orchestration/spmd_paged_attention_orch.cpptests/st/a5/tensormap_and_ringbuffer/spmd_paged_attention/test_spmd_paged_attention.pytests/st/a5/tensormap_and_ringbuffer/test_l3_dependency.pytests/st/a5/tensormap_and_ringbuffer/test_l3_group.pytests/st/a5/tensormap_and_ringbuffer/test_l3_host_buffer_registration.pytests/st/a5/tensormap_and_ringbuffer/test_prewarm_config.py
92d57d9 to
56b05a5
Compare
Two CI failures on PR hw-native-sys#1450 (st-sim-a5 / st-onboard-a5), both arch-drift from copying a2a3 sources verbatim: - Kernel compile "reference to 'Stride' is ambiguous" on a5 (Incore compile exit 1): a5 kernel headers put another Stride in scope, so bare Stride<...> is ambiguous. Qualify to pto::Stride<...> (a5 convention) in alternating_matmul_add, batch_paged_attention, paged_attention_unroll_4dims, spmd_paged_attention kernels. - L3 TypeError "missing required keyword-only argument": upstream main made Orchestrator.submit_next_level(worker=) / submit_next_level_group(workers=) required. Pass worker=0 / workers=[0, 1], matching the a2a3 originals. Verified: both L3 tests pass on a5sim; all 4 scenes' kernels compile clean for a5 (device-independent compile check).
dbf4be0 to
331ce5a
Compare
Port a2a3-only TMR scene and L3 tests to a5, omit spmd_paged_attention_highperf, and fix a5 build/runtime drift (pto::Stride qualification, required worker ids). Guard alternating_orch (a5 + a2a3) against non-positive batch sizes and non-divisible task counts: log an error and skip submission instead of a div-by-zero or silently dropping the remainder tasks. Correct the a5 L3 host-buffer test docstring to reference a5sim/a5 rather than the copied a2a3 text.
331ce5a to
d665dbd
Compare
Summary
tensormap_and_ringbufferscene/L3 tests onto a5 for differential validation (run a2a3's corpus on a5 to surface a2a3-pass/a5-fail gaps).aicpu_orchestration_entry→build_paged_attention_graph; oobset_block_num→set_core_num.spmd_paged_attention_highperffor now: fused CCE body is dav-c220-only and needs a dedicated dav-c310 port before a5 onboard is meaningful.Test plan
python -m pytest tests/st/a5/tensormap_and_ringbuffer/ --platform a5sim -vfor the newly added casestask-submitfor non-manual a5 cases, especiallyspmd_batch_dispatch_oobspmd_paged_attentionremains onboard-only (MIX not sim-buildable), matching a2a3