Fix refinement next to refined blocks #928
Conversation
During re-meshing, track which meshblocks are newly created. After prolongating shared boundaries, but before prolongating internal boundaries, update elements on block faces to respect the ownership model. Still needs tweaks to the new-mesh priority!
|
@par-hermes format |
src/mesh/mesh.cpp
Outdated
|
|
||
| // Output variables in use in this run | ||
| if (Globals::my_rank == 0) { | ||
| std::cout << "#Variables in use:\n" << *(resolved_packages) << std::endl; | ||
| } |
There was a problem hiding this comment.
Isn't this included in another PR? Did that not get merged?
There was a problem hiding this comment.
No, that one's still in queue. I pinged some people, probably it will still go in before this one and I can avoid having to correct my mistake here.
| return 2 * a.level(); | ||
| }; | ||
|
|
||
| auto ownership_less_than = [ownership_level](const LogicalLocation &a, |
There was a problem hiding this comment.
As in, I should collapse it? This seemed pretty easy to understand, if a little over-engineered
There was a problem hiding this comment.
no no---it's fine. Cheeky is not a bad thing.
There was a problem hiding this comment.
no no---it's fine. Cheeky is not a bad thing.
|
Check boxes needed :) Not for this PR, but we really need a regression test |
Yeah, we need to write a vector wave equation example I think. |
src/mesh/amr_loadbalance.cpp
Outdated
| @@ -789,6 +789,7 @@ void Mesh::RedistributeAndRefineMeshBlocks(ParameterInput *pin, ApplicationInput | |||
| RegionSize block_size = GetBlockSize(); | |||
|
|
|||
| BlockList_t new_block_list(nbe - nbs + 1); | |||
| std::set<LogicalLocation> newly_refined; | |||
There was a problem hiding this comment.
I think we may want this to be a std::unordered_set for performance reasons.
There was a problem hiding this comment.
Agreed, I think, the list could potentially be long and we're checking membership.
What's a good hash function for the unordered_set? Just morton number?
Speaking of that, I think I populate this list incorrectly, since I only pull from nbs to nbe of the global block list in populating newly_refined, meaning I need to collate the lists from all ranks. So, also going to fix that.
There was a problem hiding this comment.
A hash for LogicalLocation is injected into the std:: namespace, so std::unordered_map<LogicalLocation> should just work.
Good catch. Yeah, I think you need the locations on all ranks.
|
Okay, as updated the PR now gathers new refinements by comparing the global new & old lists of blocks (I think. The variable naming wasn't completely clear to me in an MPI context and there are no relevant comments). Testing with multiple MPI ranks now. Also switched to
Pretty sure this is the simple way to solve it, but maybe I'm missing something. |
|
Would love to just merge this in the next few days if there aren't objections |
|
I'm enabling auto merge |
PR Summary
Fixes #927
Not attached to the carry-around-the-list interface here, or the cheeky double-lambda.
Note that the issue this fixes is really two issues:
Most of the code is aimed at dealing with (2), which requires determining and then syncing according to a special priority order, namely:
That is, freshly-refined blocks should own any boundaries with coarser blocks, but none of their boundaries with previously-refined blocks (and should still choose by Morton number among themselves).
Still working on getting this entirely correct, (2) and (3) are muddy in the code as posted.
PR Checklist