Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parallel and Serial Split Explicit Free Surface #2888

Merged
merged 357 commits into from
Feb 21, 2023
Merged

Conversation

simone-silvestri
Copy link
Collaborator

@simone-silvestri simone-silvestri commented Jan 31, 2023

This PR improves the currently implemented split explicit surface solver in serial mode
(partially using the algorithm in Shchepetkin & McWilliams, Ocean Modelling 9, 2005 albeit with a linear free surface) and implements single-node parallel (MultiRegion), and multi-node parallel (Distributed) split explicit free surface

In addition, quite some improvements have been made to the distributed module to allow a distributed LatitudeLongituteGrid and IBG and various comments in the MultiRegion module

Edit: MPI does not exploit CUDA-aware message passing for CuArray views, so this PR also implements buffered Halo communication for distributed models, unifying a bit of the code for Distributed and MultiRegion. This is a fundamental step to achieve the goals of the next PR which will deal with heterogenous distributed - shared models (i.e. a MultiRegionGrid of a DistributedGrid)

Edit Edit: apparently CUDA-aware MPI allows passing views of CuArrays. Buffers are still implemented for those architectures where CUDA-aware MPI is not available

Edit Edit Edit: apparently for Oceananigans that sends strided memory, buffers are crucial because

Note that derived datatypes, both contiguous and non-contiguous, are supported. However, the non-contiguous
datatypes currently have high overhead because of the many calls to cuMemcpy to copy all the pieces of the
buffer into the intermediate buffer.

(from openmpi-link)

Edit: This PR adds the possibility of distributing along a Bounded direction, and the possibility of having correct 2D parallelization with Coriolis. Previously it was not possible because corners were not communicated correctly

Doesn't have to be merged now.

(Validation cases are not necessary and will be deleted prior to merging)

@simone-silvestri simone-silvestri changed the title Distributed and Explicit Split Explicit Free Surface Parallel and Serial Split Explicit Free Surface Jan 31, 2023
@simone-silvestri
Copy link
Collaborator Author

I should have addressed all the changes except the Field{loc...}(grid) to Field(loc, grid). I will do another PR that will refactor all the instances of Field(loc, grid) to Field{loc...}(grid)

@glwagner
Copy link
Member

I should have addressed all the changes except the Field{loc...}(grid) to Field(loc, grid). I will do another PR that will refactor all the instances of Field(loc, grid) to Field{loc...}(grid)

sounds good

@simone-silvestri simone-silvestri merged commit e394bf7 into main Feb 21, 2023

# Check that the size of a bottom field are
# consistent with the size of the field
if any(size(ib.mask) .!= mask_size)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why any?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a comment at the least

Nx, Ny, Nz = size(grid)
Hx, Hy, Nz = halo_size(grid)

mask_size = (Nx, Ny, Nz) .+ 2 .* (Hx, Hy, Hz)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the same as total_size(grid)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important to use the functions here. Remember you want to change the size of a Center, Bounded field. When you do that, the above code is wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want total_size((Center, Center, Nothing), grid) via

total_size(loc, grid) = (total_length(loc[1], topology(grid, 1), grid.Nx, grid.Hx),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed 🕸️ Our plan for total cluster domination feature 🌟 Something new and shiny GPU 👾 Where Oceananigans gets its powers from
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants