Cabana icon indicating copy to clipboard operation
Cabana copied to clipboard

Periodic index spaces

Open streeve opened this issue 1 year ago • 7 comments

Add local grid index space generation for periodic boundaries that matches the current boundary index space concept. This completes all current boundary types:

  • System boundary (non-periodic): boundaryIndexSpace
  • Periodic "system boundary": periodicIndexSpace
  • MPI boundary (not a system boundary): sharedIndexSpace

TODO: add tests to avoid issues similar to #615 due to assumed "equivalent" boundary spaces

Closes #762

streeve avatar Aug 09 '24 13:08 streeve

@JStewart28 I still need to add tests, but wanted to push this and make sure this was in the right direction

streeve avatar Aug 09 '24 13:08 streeve

sharedIndexSpace and boundaryIndexSpace look good. Question about periodicIndexSpace: If the offset is in a shared index space along a periodic boundary, boundaryIndexSpace returns, for example, for i min/max and j min/max: (0, 2), (9, 12), and periodicIndexSpace returns (0, 2), (9, 11) for the same offset. Why is the upper bound for periodicIndexSpace one lower?

JStewart28 avatar Aug 12 '24 19:08 JStewart28

sharedIndexSpace and boundaryIndexSpace look good. Question about periodicIndexSpace: If the offset is in a shared index space along a periodic boundary, boundaryIndexSpace returns, for example, for i min/max and j min/max: (0, 2), (9, 12), and periodicIndexSpace returns (0, 2), (9, 11) for the same offset. Why is the upper bound for periodicIndexSpace one lower?

I found a mistake here - initially I wrote returned the boundaryIndexSpace, but by definition the periodicIndexSpace is just a sharedIndexSpace which happens to be on a boundary (it's shared with a neighbor rank). But I'm confused on your comparison (which may or may not be because of my previous mistake): isn't any given boundary either a periodic or non-periodic, where one of the two index spaces is empty?

streeve avatar Aug 20 '24 15:08 streeve

sharedIndexSpace and boundaryIndexSpace look good. Question about periodicIndexSpace: If the offset is in a shared index space along a periodic boundary, boundaryIndexSpace returns, for example, for i min/max and j min/max: (0, 2), (9, 12), and periodicIndexSpace returns (0, 2), (9, 11) for the same offset. Why is the upper bound for periodicIndexSpace one lower?

I found a mistake here - initially I wrote returned the boundaryIndexSpace, but by definition the periodicIndexSpace is just a sharedIndexSpace which happens to be on a boundary (it's shared with a neighbor rank). But I'm confused on your comparison (which may or may not be because of my previous mistake): isn't any given boundary either a periodic or non-periodic, where one of the two index spaces is empty?

What I mean is for the same halo distance - in this case, 2 cells - and on a periodic boundary, in a periodicIndexSpace the j indices are 9 and 10, but in a sharedIndexSpace the j indices are 9, 10, and 11. sharedIndexSpace has the behavior we intend. Shouldn't the ghost index space be the same size for shared, periodic spaces whether or not they are on only an MPI boundary or an MPI+periodic boundary? Please correct me if I am missing something.

JStewart28 avatar Sep 04 '24 19:09 JStewart28

@JStewart28 I finally got back to this - I fixed one more issue. Let me know if you still see any problems, but the case I'm comparing seems to match (in the tutorial I modified here)

streeve avatar Oct 09 '24 19:10 streeve

Still trying to work this out. In Beatnik, we have the following code to give us the periodic index space for 2D partitioning:

template <class LocalGridType, class DecompositionType, class EntityType>
Cabana::Grid::IndexSpace<2>
periodicIndexSpace(LocalGridType local_grid, DecompositionType dt, EntityType et, std::array<int, 2> dir)
{
    auto & global_grid = local_grid->globalGrid();
    for ( int d = 0; d < 2; d++ ) {
        if ((dir[d] == -1 && global_grid.onLowBoundary(d))
            || (dir[d] == 1 && global_grid.onHighBoundary(d))) {
            return local_grid->sharedIndexSpace(dt, et, dir);
        }
    }
    std::array<long, 2> zero_size;
    for ( std::size_t d = 0; d < 2; ++d )
        zero_size[d] = 0;
    return Cabana::Grid::IndexSpace<2>( zero_size, zero_size );
}

Which I'm hoping becomes a 1-1 substitution for local_grid->periodicIndexSpace, but something is still off. I added this function in as another example in example/grid_tutorial/06_local_grid/local_grid_example.cpp, and sometimes the output matches boundaryIndexSpace and sometimes it matches periodicIndexSpace.

JStewart28 avatar Oct 14 '24 19:10 JStewart28

Has there been any progress on this PR?

HahsFilip avatar Sep 05 '25 17:09 HahsFilip