[Maintain][Manifest] flip Var/Std/VarMean fwd ops to status: implemented by lcy-seso · Pull Request #1430 · tile-ai/TileOPs

lcy-seso · 2026-05-12T02:58:11Z

Closes #1400

Summary

Flip VarFwdOp, StdFwdOp, VarMeanFwdOp from status: spec-only to status: implemented in tileops/manifest/reduction.yaml.
Diff is YAML-only; no signature/roofline/workloads/shape-rule edits (status flip carve-out).

Context

The issue body references impl PR #1397, which does not exist. The actual implementation already landed on main:

dim=None support via [Feat][Reduction] Support dim=None in all reduction ops for full reduction #834.
Cross-dim × correction conformance coverage via [Test][Reduction] add var/std/var_mean conformance across dim x correction #1406.

This PR is the spec-side flip aligning manifest status with the already-merged implementation and conformance tests.

Test plan

AC-1: tileops/manifest/reduction.yaml shows status: implemented for VarFwdOp, StdFwdOp, VarMeanFwdOp.
AC-2: python scripts/validate_manifest.py exits 0 (All manifest checks passed.).
AC-3: pytest tests/test_validate_manifest.py passes (218 passed, 3 warnings).
AC-4: PR diff touches only tileops/manifest/reduction.yaml (+3 / -3).

lcy-seso · 2026-05-12T02:58:50Z

/gemini review

Copilot

Pull request overview

This PR aligns the reduction manifest with the already-landed implementation and conformance coverage by promoting the variance-family forward ops (VarFwdOp, StdFwdOp, VarMeanFwdOp) from status: spec-only to status: implemented in tileops/manifest/reduction.yaml.

Changes:

Flip VarFwdOp manifest status to implemented.
Flip StdFwdOp manifest status to implemented.
Flip VarMeanFwdOp manifest status to implemented.

gemini-code-assist

Code Review

This pull request updates the status of VarFwdOp, StdFwdOp, and VarMeanFwdOp to implemented in the reduction manifest. However, several issues were identified in the underlying implementation: an AttributeError in roofline calculations due to incorrect dtype attribute access, incorrect handling of empty dimension lists in shape rules, and potential runtime errors related to non-contiguous tensor reshaping and type handling for dimension info.

gemini-code-assist · 2026-05-12T03:00:22Z

  ref_api: "torch.var"
  family: reduction
-  status: spec-only  # dim=None not yet implemented; update when impl lands
+  status: implemented


While flipping the status to implemented is correct, I noticed a bug in the underlying implementation that will affect these ops. In tileops/ops/reduction/reduce.py, the eval_roofline method (line 180) attempts to access self.dtype.itemsize. However, torch.dtype objects do not have an itemsize attribute; the correct method is element_size(). This will cause an AttributeError when calculating roofline metrics for these newly implemented ops.

Verified false positive. torch.dtype.itemsize has existed since torch 2.1, and pyproject.toml pins torch>=2.1.0,<2.11.0. Local check on torch 2.9.1: torch.float16.itemsize == 2, torch.int64.itemsize == 8 — no AttributeError. No fix needed.

gemini-code-assist

Code Review

This pull request updates the status of VarFwdOp, StdFwdOp, and VarMeanFwdOp from spec-only to implemented in the reduction manifest, indicating that these operations (torch.var, torch.std, and torch.var_mean) are now fully supported. I have no feedback to provide as there were no review comments.

Ibuki-wind

Overall

The status flips are blocked because default construction still reduces the last axis while the manifest contract says dim=None full reduction.

lcy-seso · 2026-05-12T05:01:48Z

Reviewer-feedback verification summary

After re-reading every comment on PRs #1428 / #1430 / #1431 and running targeted checks, the verified picture is:

Item	Verdict	Action
Constructor default `dim=-1` vs manifest `default: null` (Sum/Mean/Amax/Amin/Var/Std/VarMean/All/Any/CountNonzero)	✅ Real	Code-only prereq PR — flip default to `None` on `_ReduceOpBase`, `_WelfordReduceOp`, `AllFwdOp`, `AnyFwdOp`, `CountNonzeroFwdOp`; add explicit `dim=-1` override on `ProdFwdOp` (its manifest type is `int, default: -1`).
`_logical_reduce_kernel` tail-row OOB on aligned-N path (All/Any/CountNonzero)	❌ False positive	`compute-sanitizer --tool memcheck` clean across 16 shape configs (M ∈ {129, 257, 513, 1025} × N ∈ {128, 256, 512, 1024}); TileLang's `T.copy` inserts implicit row-bounds guards from the source extent. No fix needed.
`self.dtype.itemsize` `AttributeError` (gemini)	❌ False positive	`torch.dtype.itemsize` exists since torch 2.1; `pyproject.toml` pins `torch>=2.1.0,<2.11.0`.
`x.reshape(M, N)` non-contiguous risk after `flatten_for_multidim` (gemini)	❌ False positive	`flatten_for_multidim` already does `.permute(perm).contiguous()` before reshape.
`_reshape_output` not handling `tuple` (gemini)	❌ False positive	`normalize_dim` always returns `list[int]`; `tuple` is unreachable through the public API.
`shape_rules` `dim=[]` semantics, missing dtypes in signature, `hidden-state-reduce` workload missing `dim`	⚠️ Real, manifest-only	Outside the manifest-trust-model status-flip carve-out. Follow-up manifest-only PR will fix all three families together.

Plan:

Open a code-only prereq PR ([Fix][Ops] reduction op constructor defaults: dim=-1 → dim=None) covering the 5 default flips + ProdFwdOp override + default-construction test updates.
Once that lands, rebase [Maintain][Manifest] flip reduction Sum/Mean/Amax/Amin to implemented #1428 / [Maintain][Manifest] flip Var/Std/VarMean fwd ops to status: implemented #1430 / [Maintain][Manifest] flip All/Any/CountNonzero reduction ops to implemented #1431 and the status-flip claims become contract-true.
Separately, file a manifest-only PR (signature dtype lists, shape_rules empty-list semantics, hidden-state-reduce workload) for the items the trust-model carve-out won't allow here.

Detailed per-comment verdicts are posted as inline replies on each thread.

Ibuki-wind

Overall

The status flip overclaims implementation conformance; scalar input support is still missing for the flipped PyTorch variance reduction APIs.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Ibuki-wind

Overall

These flips are still ahead of the implementation: the shared reduction base rejects 0-D tensors, but torch.var / torch.std / torch.var_mean accept scalar inputs and return 0-D outputs.

Op implementations (incl. dim=None full reduction) and conformance tests are in place; promote the three Welford reductions from spec-only to implemented and drop the now-stale "not yet implemented" comments. Co-Authored-By: Ibuki 🍃 — a wind born from GPTs <[email protected]>

Ibuki-wind

Clean — no issues.

…mented (#1431) Closes #1401 ## Summary - Flip `AllFwdOp`, `AnyFwdOp`, `CountNonzeroFwdOp` from `status: spec-only` to `status: implemented` in `tileops/manifest/reduction.yaml`. - Single-file manifest-only change; no signature/roofline/workloads/shape_rules edits. ## Context Third of a sibling set: - #1428 — issue #1399, Sum/Mean/Amax/Amin. - #1430 — issue #1400, Var/Std/VarMean. - This PR — issue #1401, All/Any/CountNonzero. After all three merge, the remaining 7 spec-only reduction ops flip to `implemented`, bringing the reduction family to 19/19 `status: implemented`. ## Note on AC-5 AC-5 in the linked issue says "All 13 reduction.yaml ops are status: implemented after merge." The literal count is stale — `reduction.yaml` currently holds 19 ops (12 already `implemented` on `upstream/main`, 7 `spec-only` before this sibling set). The substantive intent — "reduction family fully implemented after merge" — is satisfied once #1428, #1430, and this PR all land. ## Test plan - [x] AC-1: `tileops/manifest/reduction.yaml` shows `status: implemented` for AllFwdOp, AnyFwdOp, CountNonzeroFwdOp. - [x] AC-2: `python scripts/validate_manifest.py` exits 0. - [x] AC-3: `pytest tests/test_validate_manifest.py` passes. - [x] AC-4: PR diff touches only `tileops/manifest/reduction.yaml` (3 lines changed). - [x] AC-5: Reduction family fully `implemented` after the three sibling PRs merge (literal "13" is a stale count; see note above). Co-authored-by: Ibuki 🍃 — a wind born from GPTs <[email protected]>

Copilot AI review requested due to automatic review settings May 12, 2026 02:58

github-actions Bot added the maintain Ongoing monitoring, health tracking, and operational maintenance label May 12, 2026

lcy-seso added automated PR produced by an autonomous agent pipeline needs-review Awaiting human review before merge labels May 12, 2026

Copilot started reviewing on behalf of lcy-seso May 12, 2026 02:58 View session

lcy-seso marked this pull request as ready for review May 12, 2026 02:59

lcy-seso requested a review from a team May 12, 2026 02:59

Copilot AI reviewed May 12, 2026

View reviewed changes

gemini-code-assist Bot reviewed May 12, 2026

View reviewed changes

lcy-seso mentioned this pull request May 12, 2026

[Maintain][Manifest] flip All/Any/CountNonzero reduction ops to implemented #1431

Merged

5 tasks

Ibuki-wind requested changes May 12, 2026

View reviewed changes

Comment thread tileops/manifest/reduction.yaml

Comment thread tileops/manifest/reduction.yaml

Comment thread tileops/manifest/reduction.yaml

lcy-seso mentioned this pull request May 12, 2026

[Maintain][Manifest] flip reduction Sum/Mean/Amax/Amin to implemented #1428

Merged

4 tasks

This was referenced May 12, 2026

[Fix][Reduction] align op defaults and empty-dim policy with manifest spec #1434

Closed

[Fix][Reduction] align constructor dim defaults + dim=[] noop on All/Any #1435

Merged

lcy-seso force-pushed the fix/manifest/issue-1400 branch from 2f0c5c2 to b7b0d27 Compare May 12, 2026 12:21

Ibuki-wind requested changes May 13, 2026

View reviewed changes

Comment thread tileops/manifest/reduction.yaml

Ibuki-wind mentioned this pull request May 13, 2026

[Manifest] align reduction status flips with scalar tensor behavior #1441

Closed

lcy-seso mentioned this pull request May 13, 2026

[Fix][Reduction] support scalar tensor input on status-implemented reduction ops #1442

Closed

7 tasks

Copilot AI review requested due to automatic review settings May 14, 2026 03:09

lcy-seso force-pushed the fix/manifest/issue-1400 branch from b7b0d27 to e048b8c Compare May 14, 2026 03:10

Copilot started reviewing on behalf of lcy-seso May 14, 2026 03:10 View session

Copilot AI reviewed May 14, 2026

View reviewed changes

Ibuki-wind requested changes May 14, 2026

View reviewed changes

Comment thread tileops/manifest/reduction.yaml

lcy-seso force-pushed the fix/manifest/issue-1400 branch from e048b8c to 3dd3edd Compare May 14, 2026 05:29

Ibuki-wind approved these changes May 14, 2026

View reviewed changes

lcy-seso merged commit b0ddd46 into tile-ai:main May 14, 2026
11 checks passed

lcy-seso deleted the fix/manifest/issue-1400 branch May 14, 2026 06:17

Conversation

lcy-seso commented May 12, 2026

Summary

Context

Test plan

Uh oh!

lcy-seso commented May 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

lcy-seso May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Ibuki-wind left a comment

Choose a reason for hiding this comment

Overall

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lcy-seso commented May 12, 2026

Uh oh!

Ibuki-wind left a comment

Choose a reason for hiding this comment

Overall

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Ibuki-wind left a comment

Choose a reason for hiding this comment

Overall

Uh oh!

Uh oh!

Ibuki-wind left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants