WIP: Refactor and extend IndexSplit to work with non-cell centered index spaces - #1388
WIP: Refactor and extend IndexSplit to work with non-cell centered index spaces#1388lroberts36 wants to merge 11 commits into
IndexSplit to work with non-cell centered index spaces#1388Conversation
This test suite captures the current IndexSplit behavior across multiple configurations (ndim, block sizes, ghost zones, chunking parameters). It will serve as regression tests during the refactoring to add topology awareness. Test coverage includes: - 1D, 2D, and 3D configurations - Multiple block sizes (4, 6, 16) - Asymmetric blocks (4x8x16) - Ghost zone counts (0, 2) - Various nkp/njp combinations (all_outer, no_outer, 0, specific values) - Both IndexDomain and explicit IndexRange constructors All public methods are tested: - outer_size(), GetBoundsK/J/I(), GetInnerBounds() - get_i(), get_deltaj() - get_max_ni/nj/nk/nij() - is_i_ghost(), is_j_ghost(), is_k_ghost(), is_ghost() Invariant checks verify: - Complete (k,j) domain coverage with no gaps or overlaps - Total work across chunks equals domain volume - Both constructors produce identical results Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Enhance test_index_split.cpp with: 1. Coverage tests using proper loop structure: - Triple-nested loop: outer (team) → k (serial) → inner (fused j-i) - Track every logical (k,j,i) point visited - Verify each interior point visited exactly once - Account for ghost cells in fused j-i loop for memory contiguity - Verify: total_iterations = interior_iterations + ghost_iterations 2. Gold file regression tests: - Record exact IndexSplit structure for representative configurations - Captures outer_size, bounds, and inner loop sizes - Generated with GENERATE_GOLD=1 environment variable - Locks down current behavior before refactoring 3. Expanded test matrix: - Focus on nghost>0 cases (where j-fusion matters) - Cover various nkp/njp combinations - Test j-fusion patterns (full, partial, none) - Asymmetric block sizes This completes Phase 1a (comprehensive testing) and Phase 1b (baseline) of the IndexSplit refactor plan. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Rename fields for clarity - Switch to indexers where possible
- add Indexer::Extent and Indexer::IdxInRange - remove unused members - integrate topological element support - start on compatibility with raw memory indexer All IndexSplit tests still passing
- Add middle loop abstraction - Add RawMemoryIndexer interface support
Yurlungur
left a comment
Yurlungur
left a comment
There was a problem hiding this comment.
I didn't read super carefully but looks good enough for government work. Thanks for filling this in!
| static IndexSplit RawMemIJ(IndexDomain domain, int halo, MeshData<Real> *md, | ||
| TE logical_te, TE memory_te = TE::CC); | ||
| static IndexSplit RawMemIJ(IndexDomain domain, MeshData<Real> *md, TE logical_te, | ||
| TE memory_te = TE::CC) { |
There was a problem hiding this comment.
I like the distinction between logical and memory topological element.
| ib.s -= halo; | ||
| ib.e += halo; |
| auto mjb = md->GetBoundsJ(IndexDomain::entire, te_mem); | ||
| auto mkb = md->GetBoundsK(IndexDomain::entire, te_mem); | ||
|
|
||
| memory_ = Indexer3D(mkb, mjb, mib); |
| target_k_ = (1.0 * total_k) / nkp_ + 1.e-6; | ||
| target_j_ = (1.0 * total_j) / njp_ + 1.e-6; | ||
|
|
||
| // save the "entire" ranges | ||
| // don't bother save ".s" since it's always zero | ||
| auto ib = md->GetBoundsI(IndexDomain::entire); | ||
| auto jb = md->GetBoundsJ(IndexDomain::entire); | ||
| auto kb = md->GetBoundsK(IndexDomain::entire); | ||
| kbe_entire_ = kb.e; | ||
| jbe_entire_ = jb.e; | ||
| ibe_entire_ = ib.e; |
There was a problem hiding this comment.
is this not needed anymore?
There was a problem hiding this comment.
yeah, everything should be available from the two 3D indexers (e.g. kbe_entire_ == memory_.Extent<KDIM>() and a lot of the logic for moving between the index spaces is already taken care of in the Indexer class. I don't exactly like how calling those sorts of functions all over the place looks, but it does mean there is one source of truth and everything is consistent. I switched to that model mostly because that was how I wrote the RawMemoryIndexer and it made it basically a one line change to switch between topological types. It also makes it pretty easy to move to spans in logical space that aren't multiples of ni (although I doubt that is too important to have).
I switched to purely integer based arithmetic for splitting up the tiled space (hence the removal of target_k_ and target_j_). It should give exactly the same results as the floating point based choices.
3 participants
PR Summary
TODO:
RawMemoryIndexerinterfacePR Checklist
// This file was made in part with generative AI.