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

feat: add halo start indices to partition_metadata #67

Merged
merged 5 commits into from
Nov 8, 2024

Conversation

TomMelt
Copy link
Contributor

@TomMelt TomMelt commented Nov 7, 2024

In order to send halo regions via MPI in the dynamics, we need to know the size of the domains and the starting indices of the halo regions.

This PR adds functionality we need to parallelize nextsimdg dynamics.

The following improvements have been made.

  • minor tweaks to DomainUtils to add helper methods
    • get_width
    • get_height
  • added Edge enums and array edges for more readable loops over neighbours
  • reworked neighbour checking to be more consistent between periodic and non-periodic neighbours
  • add functionality to compute the starting indices for the halo regions in neighbouring domains
  • replaced loops over NNBRS to use the new edges enums
  • minor fix - ensure that std::cerr messages are followed by exit(EXIT_FAILURE).

TODO

  • update integration tests to reflect additional outputs (halo_starts)

@TomMelt TomMelt added enhancement New feature or request ICCS labels Nov 7, 2024
@TomMelt TomMelt requested a review from jwallwork23 November 7, 2024 12:23
@TomMelt TomMelt self-assigned this Nov 7, 2024
@TomMelt TomMelt marked this pull request as draft November 7, 2024 13:00
- minor tweaks to `DomainUtils` to add helper methods
  - `get_width`
  - `get_height`
- added `Edge` enums and array `edges` for more readable loops over
  neighbours
- reworked neighbour checking to be more consistent between periodic and
  non-periodic neighbours
- add functionality to compute the starting indices for the halo regions
  in neighbouring domains
- replaced loops over `NNBRS` to use the new `edges` enums
- minor fix - ensure that `std::cerr` messages are followed by
  `exit(EXIT_FAILURE)`.
@TomMelt TomMelt force-pushed the add-halo-start-indices branch from e040578 to fc4afae Compare November 7, 2024 16:53
@TomMelt TomMelt marked this pull request as ready for review November 7, 2024 16:59
Copy link
Contributor

@jwallwork23 jwallwork23 left a comment

Choose a reason for hiding this comment

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

Thanks @TomMelt. I have a few minor comments and a request for more information. Could you elaborate on what exactly is meant by "add functionality to compute the starting indices for the halo regions in neighbouring domains"?

@TomMelt TomMelt force-pushed the add-halo-start-indices branch from f810ef9 to 8bdc0fe Compare November 8, 2024 12:26
Copy link
Contributor

@jwallwork23 jwallwork23 left a comment

Choose a reason for hiding this comment

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

Thanks for these edits, things are much clearer now. Just one minor comment then I think it's good to go!

@TomMelt TomMelt merged commit 9efa78b into main Nov 8, 2024
3 checks passed
@TomMelt TomMelt deleted the add-halo-start-indices branch November 8, 2024 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ICCS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants