[STF] Implement cyclic_partition::get_executor#9803
Conversation
cyclic_partition::get_executor was left as abort(), making cyclic the only partitioner unusable as a composite data place mapper, which needs the inverse coordinate-to-place direction. Implement the exact inverse of apply for zero-based coordinates: the owner of an element is its coordinate modulo the grid extent, independently in each dimension. Add a co-located unittest sweeping every place of a 2x3 grid over a 3-D box and checking that each coordinate visited by apply maps back to the place that produced it, covering dimensions the grid does not split. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesCyclic executor ownership
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cudax/include/cuda/experimental/__places/partitions/cyclic_shape.cuh (1)
385-386: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winsuggestion: Track visited coordinates, not only the count.
cnt == 7 * 5 * 20allows duplicates to mask missing elements. Record each coordinate and assert uniqueness so the test actually verifies complete coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c040d1cb-764f-4a09-b6ef-34182eca0a42
📒 Files selected for processing (1)
cudax/include/cuda/experimental/__places/partitions/cyclic_shape.cuh
A hand-built dim4 can carry zero extents, which would make the modulo undefined; real grids already assert positive dimensions, so this only guards direct misuse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/ok to test 5a28243 |
🥳 CI Workflow Results🟩 Finished in 1h 06m: Pass: 100%/57 | Total: 1d 00h | Max: 1h 06m | Hits: 14%/178813See results here. |
1 participant
Description
cyclic_partition::get_executorwas left asabort(), making cyclic the only partitioner unusable as a composite data place mapper (which needs the inverse coordinate-to-place direction:data_place::composite(cyclic_partition{}, grid)aborted at the first allocation).This implements the exact inverse of
applyfor zero-based coordinates — the owner of an element is its coordinate modulo the grid extent, independently in each dimension — matching the zero-based contract of the siblingblocked_partition/tiled_partitionexecutors, with a debug assert rejecting negative coordinates.A co-located unittest sweeps every place of a 2×3 grid over a 3-D box and checks that each coordinate visited by
applymaps back to the place that produced it, covering dimensions the grid does not split.Checklist
🤖 Generated with Claude Code