test(rfc-1300): migrate in-tree tests off pl.Tensor[..., pl.DN] shorthand (Q4 follow-up)#1354
Conversation
…hand (Q4) Removes the deprecated ``pl.Tensor[..., pl.DN]`` shorthand from every in-tree test that wasn't specifically validating the DN codegen path. Migration patterns applied: * **B^T-matmul marker** — drop the DN annotation; keep the ``transpose=True`` on the matching ``pl.load``. The Q5 path (ConvertTensorToTileOps lowering of transpose=True loads) is the intended replacement — annotating the param adds no information beyond what the load already encodes. Affected: ``test_expand_mixed_kernel_a5.py`` (4 sites), ``test_pto_codegen_paged_attn.py``, ``test_paged_attention.py``. * **DN-specific codegen tests** — keep the DN view but use the canonical-stride ``pl.TensorView(stride=[...], layout=DN)`` form (RFC hw-native-sys#1300 supplementary 1 escape hatch). The DN view is the test subject here, not an alias — so it must stay, but in the explicit stride form rather than the deprecated shorthand. Affected: ``test_pto_codegen.py`` (``test_column_vector_with_explicit_dn``, ``test_pto_codegen_3d_dn_tensor_view_uses_canonical_stride``). * **Parser tests for the shorthand itself** — the ``pl.DN`` parsing path still exists for backward compatibility, so the tests now assert the ``DeprecationWarning`` fires rather than silently accept the layout. Affected: ``test_type_resolver.py`` — ``test_resolve_tensor_with_dn_layout_warns``, ``test_function_with_dn_layout_warns``. Other parametrized cases drop the DN entry (covered by the dedicated tests). * **Deleted test** — ``test_already_dn_param_idempotent`` in ``test_lower_transpose_load_param_layout_pass.py``. The "user wrote a DN-tagged param" input pattern is exactly what Q4 deprecates; the pass's short-circuit branch is now unreachable from valid DSL. Existing no-op coverage in ``test_no_transpose_unchanged`` plus the full TestNoOpCases / TestStridedParamFlipsCorrectly suite still exercises every reachable code path. After this commit, ``grep -rE 'pl\.DN|pl\.Tensor\[.*pl\.DN' tests/`` returns zero hits — every remaining ``pl.TensorLayout.DN`` reference is either an explicit ``pl.TensorView(stride=..., layout=DN)`` (the supported form) or a printer/parser assertion about the layout enum itself. All 4630 unit tests pass.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request deprecates the pl.DN shorthand for tensor layouts across the test suite, aligning with RFC #1300. Changes include removing the shorthand from various tests, replacing it with explicit pl.TensorView in codegen tests, and adding verification for DeprecationWarning in the type resolver. Feedback was provided regarding the deletion of a test case in test_lower_transpose_load_param_layout_pass.py, which leaves a logic branch unexercised; it is recommended to refactor this test to use the explicit pl.TensorView form instead of removing it entirely.
I am having trouble creating individual review comments. Click here to see my feedback.
tests/ut/ir/transforms/test_lower_transpose_load_param_layout_pass.py (444-481)
While the pl.Tensor[..., pl.DN] shorthand is deprecated, the LowerTransposeLoadParamLayout pass still contains logic to handle parameters that are already DN-tagged (e.g., from an explicit pl.TensorView). Deleting test_already_dn_param_idempotent leaves the if (view.layout == TensorLayout::DN) continue; branch in the pass unexercised. Consider refactoring this test to use the explicit pl.TensorView form instead of deleting it. Additionally, ensure the test function is explicitly annotated with the target function type (e.g., @pl.function(type=pl.FunctionType.InCore)) to ensure it is not skipped by the pass.
References
- When writing unit tests for IR passes, ensure the test functions are explicitly annotated with the target function type (e.g., InCore) to prevent them from being treated as Opaque and skipped.
…hand (Q4 follow-up) (hw-native-sys#1354)
2 participants
Summary
Follow-up to #1347. Migrates every in-tree test that still used the deprecated
pl.Tensor[..., pl.DN]shorthand — so the newDeprecationWarningintroduced in #1347 fires only at the deprecation sites we explicitly assert it at, not as background noise across unrelated tests.After this PR,
grep -rE 'pl\.DN|pl\.Tensor\[.*pl\.DN' tests/returns zero hits.Migration patterns
Three classes of sites, three migration patterns:
B^T-matmul markers — the DN annotation was just a hint that the param feeds a
transpose=Trueload. The load already encodes the transpose; the annotation adds nothing. Drop the marker, keep the source shape.Sites:
test_expand_mixed_kernel_a5.py(4×),test_pto_codegen_paged_attn.py,test_paged_attention.py(st).DN-specific codegen tests — the DN view is the test subject (column-vector codegen, 3-D DN canonical-stride codegen). Keep the DN view but switch to the explicit-stride
pl.TensorView(stride=[...], layout=DN)form (RFC [RFC] Self-consistent IR TensorType layout representation #1300 supplementary 1 escape hatch).Sites:
test_pto_codegen.py::test_column_vector_with_explicit_dn,test_pto_codegen_3d_dn_tensor_view_uses_canonical_stride.Parser tests that specifically exercise the shorthand — backward compat still resolves
pl.DNto a DN-tagged TensorView, so the tests now assert theDeprecationWarningfires rather than silently accept the resolution.Sites:
test_type_resolver.py::test_resolve_tensor_with_dn_layout_warns,test_function_with_dn_layout_warns. Parametrized lists drop the DN row (covered by the dedicated tests).Deleted test —
test_already_dn_param_idempotentintest_lower_transpose_load_param_layout_pass.py. That test specifically validated the pass's behavior when a user wrote a DN-tagged param — exactly the input pattern feat: deprecate pl.Tensor[..., pl.DN] + strict TensorViewCanonical (RFC #1300 §2.4 + supplementary 1) #1347 deprecates. The pass's short-circuit branch is no longer reachable from valid DSL; existingTestNoOpCasesandTestStridedParamFlipsCorrectlycoverage still exercises every reachable code path.Test plan
4655 passed, 41 skipped(locally on macOS arm64).test_type_resolver.py, not as background warnings across unrelated tests.test_column_vector_with_explicit_dn,test_pto_codegen_3d_dn_tensor_view_uses_canonical_stride) still pass — the canonical-stride TensorView reaches codegen with the same effective view shape the shorthand produced.