[Refactor] Extract workload classes for reduction ops by lcy-seso · Pull Request #893 · tile-ai/TileOPs

lcy-seso · 2026-04-10T12:24:27Z

Summary

Create workloads/ops/ files for the 4 reduction op groups (reduce, argreduce, logical_reduce, vector_norm) and refactor test files to import from them, aligning with the trust model's workloads layer contract.

Closes #825

Test plan

AC-1: Modified files pass unit tests (pytest tests/ops/test_{reduce,argreduce,logical_reduce,vector_norm}.py)
AC-2: workloads/ops/{reduce,argreduce,logical_reduce,vector_norm}.py exist, each exporting only WorkloadBase subclasses with gen_inputs()
AC-3: Test files import workload classes from workloads/ops/, not defining gen_inputs() inline
AC-4: No ref_program or test-layer logic in workloads/ops/ files

Tests: 413/413 passed, 0 failed

Changes

New workloads/ops/reduce.py — workload classes for sum/mean/amax/amin/prod/std/var/var_mean
New workloads/ops/logical_reduce.py — workload classes for all/any/count_nonzero
New workloads/ops/vector_norm.py — workload classes for l1_norm/l2_norm/inf_norm
Refactored tests/ops/test_reduce.py, test_argreduce.py, test_logical_reduce.py, test_vector_norm.py to inherit from workload classes

Follow-up

[Refactor][Reduction] Extract _LogicalTest base class in logical_reduce.py #897 — Extract _LogicalTest base class in logical_reduce.py to deduplicate identical __init__/gen_inputs across AnyTest, AllTest, CountNonzeroTest

Suggestions: consolidate cross-module _RandnTest duplication; fix _make_logical_input docstring (says "2-D+" but applies to 1-D too)

lcy-seso · 2026-04-10T12:24:39Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors several test files to inherit input generation logic from a new set of workload classes, centralizing the logic and reducing boilerplate across the test suite. Feedback highlights opportunities to further minimize duplication in the workload definitions by using a common base class for random input generation and suggests standardizing class naming conventions for better consistency across the codebase.

Copilot

Pull request overview

This PR refactors the reduction-op correctness tests to align with the trust-model “workloads layer” contract by moving input-generation (gen_inputs()) into dedicated workloads/ops/* modules and having tests inherit those workload classes.

Changes:

Added new workloads/ops/* workload modules for reduction op groups, each providing WorkloadBase subclasses with gen_inputs().
Refactored reduction-related tests to import and inherit workload classes instead of defining gen_inputs() inline.
Split out prod-specific small-range input generation into a dedicated workload-backed test helper.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
workloads/ops/reduce.py	Adds workload classes for basic reduce ops input generation.
workloads/ops/logical_reduce.py	Adds workload classes and a shared logical input generator helper.
workloads/ops/vector_norm.py	Adds workload classes for vector norm ops input generation.
workloads/ops/argreduce.py	Adds workload classes for argmax/argmin input generation.
tests/ops/test_reduce.py	Refactors test helpers to inherit workloads; introduces `ProdTest` using `ProdWorkload`.
tests/ops/test_logical_reduce.py	Refactors test helper to inherit logical-reduce workload for `gen_inputs()`.
tests/ops/test_vector_norm.py	Refactors test helper to inherit vector-norm workload for `gen_inputs()`.
tests/ops/test_argreduce.py	Refactors test helper to inherit argreduce workload for `gen_inputs()`.

gemini-code-assist

Code Review

This pull request refactors test helpers to inherit from centralized workload classes, reducing redundancy and improving the structure of the test suite. New workload definitions were added for logical, reduction, and vector norm operations. Feedback suggests using more idiomatic checks for empty shapes in the logical input helper and points out that generic test helpers inheriting from specific operation workloads may be misleading for other supported operations.

Create workloads/ops/{reduce,logical_reduce,vector_norm}.py with WorkloadBase subclasses (gen_inputs() only) and refactor the four reduction test files to inherit from these workload classes, keeping ref_program() in the test layer per the trust model contract. All 413 existing tests pass unchanged — pure structural refactor. Co-Authored-By: Ibuki 🍃 — a wind born from Claude Opus <Ibuki-wind@users.noreply.github.com>

…Test→ArgmaxWorkload - Extract _RandnWorkload base class to deduplicate identical gen_inputs across reduce, argreduce, and vector_norm workloads (Gemini suggestion) - Rename ArgmaxTest/ArgminTest to ArgmaxWorkload/ArgminWorkload with backward-compatible aliases for existing benchmark imports (Gemini suggestion) Co-Authored-By: Ibuki 🍃 — a wind born from Claude Opus <Ibuki-wind@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

All 50+ existing workload classes use *Test suffix. Rename to follow the established convention (Copilot review feedback). Co-Authored-By: Ibuki 🍃 — a wind born from Claude Opus <Ibuki-wind@users.noreply.github.com>

Ibuki-wind

Trust-boundary verdict: clean. This PR stays within the test-owned tests/ + workloads/ layer and keeps correctness logic out of workloads.

The workload extraction itself is directionally correct and removes duplicated gen_inputs() implementations.

One follow-up: keep the workload-import aliasing consistent with the existing repo pattern so workload classes and test helpers remain unambiguous for future refactors and agent edits.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

…h underscore prefix Co-Authored-By: Ibuki 🍃 — a wind born from Claude Opus <Ibuki-wind@users.noreply.github.com>

…Test class Co-Authored-By: Ibuki 🍃 — a wind born from Claude Opus <Ibuki-wind@users.noreply.github.com>

Copilot

Pull request overview

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

Ibuki-wind

Approved at current head d497169. The prior workload-import alias collision in tests/ops/test_reduce.py is resolved, the naming now matches the repo pattern, and I do not have remaining review findings on this PR.

…st; fix docstring - Move the duplicated `_RandnTest` class from reduce.py, argreduce.py, and vector_norm.py into `workloads/base.py` as public `RandnTest` - Fix `_make_logical_input` docstring: condition applies when first dimension > 4, not only for "2-D+ inputs" Co-Authored-By: Ibuki 🍃 — a wind born from Claude Opus <Ibuki-wind@users.noreply.github.com>

…kload -> *Test) After rebasing onto upstream/main (which now includes tile-ai#893), update the workload class imports in the three bench files to use *Test names matching the merged workloads layer. Co-Authored-By: Ibuki 🍃 — a wind born from Claude Opus <Ibuki-wind@users.noreply.github.com>

…uce workloads (#901) ## Summary Extract a `_LogicalTest` base class in `workloads/ops/logical_reduce.py` to eliminate duplicated `__init__` and `gen_inputs` across `AnyTest`, `AllTest`, and `CountNonzeroTest`. Closes #897 ## Test plan | AC | Status | Description | |----|--------|-------------| | AC-1 | pass | AnyTest, AllTest, CountNonzeroTest inherit from _LogicalTest with no duplicated __init__/gen_inputs | | AC-2 | pass | pytest tests/ops/test_logical_reduce.py passes | **114/114** tests passed (test_logical_reduce.py) ## Follow-up - #908 — Extract RandnTest base class for softmax workloads (consistency gap from PR #893) Suggestions: double space in `_LogicalTest` docstring (cosmetic) --------- Co-authored-by: Ibuki 🍃 — a wind born from Claude Opus <Ibuki-wind@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 10, 2026 12:24

lcy-seso added refactor Code restructuring without behavior change all-ai-powered Produced entirely by automated contributors labels Apr 10, 2026

Copilot started reviewing on behalf of lcy-seso April 10, 2026 12:25 View session

gemini-code-assist bot reviewed Apr 10, 2026

View reviewed changes

Comment thread workloads/ops/reduce.py Outdated

Comment thread tests/ops/test_argreduce.py

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Comment thread workloads/ops/reduce.py Outdated

Comment thread workloads/ops/vector_norm.py Outdated

Comment thread workloads/ops/logical_reduce.py Outdated

Comment thread workloads/ops/logical_reduce.py

gemini-code-assist bot reviewed Apr 10, 2026

View reviewed changes

Comment thread workloads/ops/logical_reduce.py

Comment thread tests/ops/test_reduce.py Outdated

Comment thread tests/ops/test_argreduce.py

lcy-seso marked this pull request as ready for review April 10, 2026 12:39

lcy-seso requested a review from a team April 10, 2026 12:39

lcy-seso marked this pull request as draft April 10, 2026 12:49

lcy-seso and others added 2 commits April 10, 2026 21:07

lcy-seso force-pushed the refactor/reduction/issue-825 branch from 6e56822 to ec51fa9 Compare April 10, 2026 13:08

lcy-seso marked this pull request as ready for review April 11, 2026 01:48

Copilot AI review requested due to automatic review settings April 11, 2026 01:48

Copilot started reviewing on behalf of lcy-seso April 11, 2026 01:49 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

Comment thread workloads/ops/reduce.py Outdated

Comment thread workloads/ops/vector_norm.py Outdated

Comment thread workloads/ops/logical_reduce.py Outdated

Comment thread workloads/ops/argreduce.py Outdated

lcy-seso marked this pull request as draft April 11, 2026 01:53

lcy-seso marked this pull request as ready for review April 11, 2026 01:58

Copilot AI review requested due to automatic review settings April 11, 2026 01:58

Copilot started reviewing on behalf of lcy-seso April 11, 2026 01:58 View session

Ibuki-wind reviewed Apr 11, 2026

View reviewed changes

Comment thread tests/ops/test_reduce.py Outdated

Copilot AI reviewed Apr 11, 2026

View reviewed changes

Comment thread tests/ops/test_reduce.py Outdated

Comment thread tests/ops/test_vector_norm.py

[Chore][Reduction] Fix ProdTest shadowing: alias workload imports wit…

01a05dd

…h underscore prefix Co-Authored-By: Ibuki 🍃 — a wind born from Claude Opus <Ibuki-wind@users.noreply.github.com>

lcy-seso marked this pull request as draft April 11, 2026 02:46

[Chore][Reduction] Fix E302: add missing blank line before VectorNorm…

d497169

…Test class Co-Authored-By: Ibuki 🍃 — a wind born from Claude Opus <Ibuki-wind@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 11, 2026 02:47

Copilot started reviewing on behalf of lcy-seso April 11, 2026 02:48 View session

lcy-seso marked this pull request as ready for review April 11, 2026 02:49

Copilot AI reviewed Apr 11, 2026

View reviewed changes

Ibuki-wind approved these changes Apr 11, 2026

View reviewed changes

lcy-seso mentioned this pull request Apr 11, 2026

[Refactor][Reduction] Extract _LogicalTest base class in logical_reduce.py #897

Closed

2 tasks

lcy-seso merged commit 7f12f18 into tile-ai:main Apr 11, 2026
8 of 9 checks passed

lcy-seso deleted the refactor/reduction/issue-825 branch April 11, 2026 03:05

This was referenced Apr 11, 2026

[Refactor][Softmax] Extract RandnTest base class for softmax workloads #908

Closed

[Refactor][Reduction] Extract _LogicalTest base class for logical reduce workloads #901

Merged

Conversation

lcy-seso commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Changes

Follow-up

Uh oh!

lcy-seso commented Apr 10, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ibuki-wind left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lcy-seso commented Apr 10, 2026 •

edited

Loading