Skip to content

Conversation

@niravshah241
Copy link

@niravshah241 niravshah241 commented Oct 20, 2025

Once the domain is divided across different MPI ranks, the rank corresponding to the neighbour of a given subdomain in a given direction needs to be identified. Currently the functionality is implemented to identify neighbours in TOP, RIGHT, BOTTOM and LEFT direction which share a common edge with the given subdomain. This Draft PR aims to add functionality to identify neighbours in TOP_RIGHT, BOTTOM_RIGHT, BOTTOM_LEFT and TOP_LEFT corners.

Specifically, the changes include:

@TomMelt
Copy link
Contributor

TomMelt commented Oct 20, 2025

@niravshah241 thanks for drafting this PR. I know that it is draft stage but draft PRs still shouldn't contain CMake build files. Please remove these as discussed last meeting. Please only commit changes for relevant source files.

@niravshah241 niravshah241 self-assigned this Oct 23, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

@niravshah241 as a rule, I would generally just remove this change. It's just formatting. Unless it's required by the linter (which I dont think it is).

…c neighbour tests in integration still need to be fixed, More tests test_3 and test_4 added for corner neighbours, test_1 and test_2 reference files updated to include details on corner neighbour
@niravshah241
Copy link
Author

niravshah241 commented Nov 7, 2025

Pending tasks:

  • Remove minor formatting related errors
  • Add one more test (say test_5) with overlap with other domain on top and bottom edge To be addressed as separately, if necessary
  • Rebasing this branch onto main
  • Change n,m to x,y in the tests
  • Add a comment to the nested functions saying something like, "this should only be called if is_neighbour is true
  • Periodic cases (PR Add periodic corner neighbours #77)
  • Add Link of Halo exchange logic
  • Unit test for is_corner_neighbour To be addressed as separately and not in this PR

* update tests to work with main branch
* update Partitioner to use new halo_send/recv approach for edges
niravshah241 added 2 commits November 17, 2025 17:02
…regarding calling is_corner_neighbour is added in halo_corner_start
@niravshah241 niravshah241 marked this pull request as ready for review November 17, 2025 19:28
}
}

bool Partitioner::is_corner_neighbour(
Copy link

Choose a reason for hiding this comment

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

Add some documentation here to explain how this works?

const Domain d1, const Domain d2, const Vertex vertex, const bool is_px, const bool is_py)
{
if (vertex == TOP_LEFT) {
// is_px and is_py is not updatd ath moment as periodic boundaries are not considered.
Copy link

Choose a reason for hiding this comment

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

Typo here.

return start;
}

int Partitioner::halo_corner_start(const Domain d1, const Domain d2, const Vertex vertex)
Copy link

Choose a reason for hiding this comment

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

Make clear that a pre-condition of using this function is that is_corner_neighbour is true.

Partitioner.cpp Outdated
Comment on lines 59 to 60
// is_px and is_py is not updatd ath moment as periodic boundaries are not considered.
// Check if TOP Left neighbour i.e.,
Copy link

Choose a reason for hiding this comment

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

Some documentation on what is_px and is_py are for is needed.
I don't understand why the TOP_LEFT case doesn't look at either is_py or is_px but all the other cases do... why would something be special here?

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.

4 participants