[Maintain][Manifest] flip reduction Sum/Mean/Amax/Amin to implemented by lcy-seso · Pull Request #1428 · tile-ai/TileOPs

lcy-seso · 2026-05-12T02:47:10Z

Closes #1399

Summary

Flip SumFwdOp, MeanFwdOp, AmaxFwdOp, AminFwdOp in tileops/manifest/reduction.yaml from status: spec-only to status: implemented.
No changes to signature, shape_rules, dtype_combos, roofline, or workloads on the flipped entries.
Implementation already landed via [FEAT][REDUCTION] implement dim=none for arithmetic reductions (sum/mean/amax/amin) #1396; this PR closes the spec/impl trust-boundary handoff.

Test plan

AC-1: tileops/manifest/reduction.yaml shows status: implemented for SumFwdOp, MeanFwdOp, AmaxFwdOp, AminFwdOp.
AC-2: python scripts/validate_manifest.py exits 0.
AC-3: pytest tests/test_validate_manifest.py passes (218 passed).
AC-4: PR diff touches only tileops/manifest/reduction.yaml.

lcy-seso · 2026-05-12T02:47:41Z

/gemini review

Copilot

Pull request overview

This PR promotes four arithmetic reduction ops in the manifest (SumFwdOp, MeanFwdOp, AmaxFwdOp, AminFwdOp) from spec-only to implemented, completing the manifest-side handoff after the corresponding implementation landed earlier. This makes the ops discoverable/dispatchable through the manifest while keeping signatures, shape rules, dtype combos, roofline, and workloads unchanged.

Changes:

Flip status to implemented for SumFwdOp and MeanFwdOp in tileops/manifest/reduction.yaml.
Flip status to implemented for AmaxFwdOp and AminFwdOp in tileops/manifest/reduction.yaml.

gemini-code-assist

Code Review

This pull request updates the status of several reduction operations, including SumFwdOp, MeanFwdOp, AmaxFwdOp, and AminFwdOp, from 'spec-only' to 'implemented' in the manifest. While the status change is appropriate, the review identifies several inconsistencies: the manifest's default 'dim' value (null) conflicts with the Python implementation's default (-1), certain workloads lack explicit dimension specifications leading to ambiguity, and the handling of empty dimension lists in shape and roofline rules does not align with PyTorch's behavior.

gemini-code-assist

Code Review

This pull request updates the status of several reduction operations in the manifest, specifically SumFwdOp, MeanFwdOp, AmaxFwdOp, and AminFwdOp, from 'spec-only' to 'implemented'. This change indicates that these operations are now fully supported, including cases where the dimension is not specified. I have no feedback to provide as there were no review comments to evaluate.

Ibuki-wind

Overall

One conformance blocker remains before this manifest can truthfully claim implemented status.

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

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 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.sum / torch.mean / torch.amax / torch.amin accept scalar inputs and return 0-D outputs.

Promote SumFwdOp, MeanFwdOp, AmaxFwdOp, AminFwdOp from spec-only to implemented now that the aligned impl supports dim=None / int / tuple across the manifest's declared dtype set. Validator and tests/test_validate_manifest.py pass. 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:47

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:47 View session

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

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

Copilot AI reviewed May 12, 2026

View reviewed changes

gemini-code-assist Bot reviewed May 12, 2026

View reviewed changes

Comment thread tileops/manifest/reduction.yaml

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

This was referenced May 12, 2026

[Maintain][Manifest] flip Var/Std/VarMean fwd ops to status: implemented #1430

Merged

[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-1399 branch from 1d140bc to 376189d 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-1399 branch from 376189d to 646c4ee Compare May 14, 2026 03:09

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-1399 branch from 646c4ee to 642099c Compare May 14, 2026 05:29

Ibuki-wind approved these changes May 14, 2026

View reviewed changes

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

[Maintain][Manifest] flip reduction Sum/Mean/Amax/Amin to implemented#1428
lcy-seso merged 1 commit into
tile-ai:mainfrom
lcy-seso:fix/manifest/issue-1399

lcy-seso commented May 12, 2026

Uh oh!

lcy-seso commented May 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Ibuki-wind left a comment

Uh oh!

Uh oh!

lcy-seso commented May 12, 2026

Uh oh!

Ibuki-wind left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Ibuki-wind left a comment

Uh oh!

Uh oh!

Ibuki-wind left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

lcy-seso commented May 12, 2026

Summary

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!

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!

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