Skip to content

Conversation

@TomMelt
Copy link
Contributor

@TomMelt TomMelt commented Dec 2, 2025

WIP

This is an initial implementation of the correct halo send positions for corner neighbours

TODO:

  • add unit tests for haloCornerBufferPositions and haloBufferPositions functions
  • fix haloCornerBufferPositions for periodic corner neighbours
  • @niravshah241 is investigating why the test_halo_corner_start test is failing

@TomMelt TomMelt requested a review from niravshah241 December 2, 2025 15:45
@TomMelt TomMelt self-assigned this Dec 2, 2025
@TomMelt TomMelt added the ICCS label Dec 2, 2025
@TomMelt TomMelt marked this pull request as draft December 2, 2025 15:46
@TomMelt TomMelt changed the base branch from main to add-periodic-corner-nbrs December 2, 2025 15:46
Copy link

@niravshah241 niravshah241 left a comment

Choose a reason for hiding this comment

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

Overall, looks good. Some quick formulae checks and making private members to public in partition.hpp have been suggested.

The unit tests for is_corner_neighbour and haloCornerBufferPositions (previously halo_corner_start) have been written in the branch add-periodic-corner-nbrs. (It will need some changes depending upon final function calls and index changes.)

Partitioner.cpp Outdated
start = left - d2.p1.x - 1;
// this second case works if the corner neighbour lies on the bottom edge (or in the
// corner of both e.g., the bottom left corner)
int dx = d1.p2.y - d2.p1.y;

Choose a reason for hiding this comment

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

Should this be dx = d1.p2.x - d2.p1.x?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. I have either named dx incorrectly, or it should be d1.p2.x. I will double check these and get back to you

Partitioner.hpp Outdated
// Vector of maps of "corner" neighbours to their halo start indices after partitioning
std::vector<std::map<int, int>> _corner_send_pos_p = std::vector<std::map<int, int>>(NNBRS);

private:

Choose a reason for hiding this comment

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

Do we necessarily need private here? If no, it would be better (at least for the unit tests) if the functions is_neighbour , is_corner_neighbour, haloBufferPositions and haloCornerBufferPositions are made public.

@TomMelt TomMelt force-pushed the fix-corner-starts branch from 0e4179e to 66cd9d5 Compare January 8, 2026 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants