TransferManager: support a redistributed AdaptiveMeshHierarchy#5215
TransferManager: support a redistributed AdaptiveMeshHierarchy#5215pbrubeck wants to merge 15 commits into
Conversation
404f925 to
381f544
Compare
381f544 to
885a7eb
Compare
| return coarse_dual | ||
|
|
||
|
|
||
| def _simplex_cell_volumes(mesh): |
There was a problem hiding this comment.
Falling back to numpy should be stressed as the worst antipattern in AGENTS.md
| parent_cell_numbers = getattr(transfer_mesh, "_adaptive_parent_cell_numbers", None) | ||
| if parent_cell_numbers is None: | ||
| parent_cell_numbers = getattr(mesh, "_adaptive_parent_cell_numbers", None) | ||
| if parent_cell_numbers is None: | ||
| parent_cell_numbers = adaptive_parent_cell_numbers(self.meshes[-1], mesh) |
There was a problem hiding this comment.
Just pick a convention on where this attribute should ultimately be set.
| def adaptive_netgen_cells(mesh): | ||
| tdim = mesh.topological_dimension | ||
| if tdim == 2: | ||
| return mesh.netgen_mesh.Elements2D() | ||
| elif tdim == 3: | ||
| return mesh.netgen_mesh.Elements3D() | ||
| raise NotImplementedError("Adaptive refinement is only implemented in dimension 2 and 3.") | ||
|
|
||
|
|
||
| def adaptive_netgen_cell_count(mesh): | ||
| return getattr(mesh, "_adaptive_netgen_num_cells", len(adaptive_netgen_cells(mesh))) |
There was a problem hiding this comment.
Do we need to get the netgen specific cell count? We should instead branch on wheter the netgen geometry is the global or local one. On refined meshes (either uniformly or adaptively refined ones) it is possible that the netgen mesh gets distributed. Which reminds me, we should add some tests that compose uniform and adaptive refiments, i.e.
mesh = Mesh(netgen_mesh)
mh = MeshHierarchy(mesh, N)
amh = AdaptiveMeshHierarchy(mh[-1])
Maybe even we should also add support for AdaptiveMeshHierarchy(mh) to populate the adaptive hierarcy with N levels of uniform refiment. In the tests N=1 or 2.
| return result | ||
|
|
||
|
|
||
| def _adaptive_parent_owner_partition(coarse_mesh, parent_cell_numbers): |
There was a problem hiding this comment.
This is effectively un-redistributing the refined mesh. We should be adaptively refining into parent-owned and then calling dm.distribute. The problem is that Mesh(refined_netgen_mesh) will redistribute it by default.
| num_cells = transfer_mesh.cell_set.size | ||
| min_cells = transfer_mesh.comm.allreduce(num_cells, op=MPI.MIN) | ||
| max_cells = transfer_mesh.comm.allreduce(num_cells, op=MPI.MAX) | ||
| needs_redist = max_cells > 1.15 * min_cells |
There was a problem hiding this comment.
We should a balancing threshold kwarg. What could be a good default value, maybe 0.5? The tests should pick something much lower to trigger redistribution, like 0.15.
| needs_redist = max_cells > 1.15 * min_cells | |
| needs_redist = max_cells > (1+balancing) * min_cells |
There was a problem hiding this comment.
A better criterion is max_cells > (1+balancing) * avg_cells
| redist_mesh_parameters["partition"] = False | ||
| redist_mesh_parameters["overlap_type"] = ( | ||
| DistributedMeshOverlapType.NONE, 0 | ||
| ) |
There was a problem hiding this comment.
Use DISTRIBUTION_PARAMETERS_NOOP
There was a problem hiding this comment.
Is there a good reason to carry over the parent's distribution parameters? It seems that we override everything about distribution. We should just pass DISTRIBUTION_PARAMETERS_NOOP to the Mesh constructor.
| return point_sf_orig, point_sf | ||
|
|
||
|
|
||
| def make_unoverlapped_dm(mesh): |
There was a problem hiding this comment.
Add a docsting with the comment that was removed, maybe make it more descriptive/clear.
| def make_unoverlapped_dm(mesh): | |
| def make_unoverlapped_dm(mesh): | |
| """Effectively "invert" addOverlap(). | |
| The resulting plex is to have the identical data structure as the one before addOverlap(). | |
| This is algorithmically guaranteed.""" |
| return dm | ||
|
|
||
|
|
||
| def fixup_embedded_coords(dm, mesh): |
There was a problem hiding this comment.
Move this back to MeshHierachy, this function is only called once.
|
|
||
| def dm_has_empty_rank(dm): | ||
| cstart, cend = dm.getHeightStratum(0) | ||
| comm = dm.comm.tompi4py() if hasattr(dm.comm, "tompi4py") else dm.comm |
There was a problem hiding this comment.
Is this logic necessary? DM should only have one type of comm.
| from firedrake.halo import MPI | ||
|
|
||
|
|
||
| class RedistMesh: |
There was a problem hiding this comment.
Move the new code to the bottom of this file
| transfer_mesh = fine_mesh | ||
| flgmaps = ( | ||
| fine_lgmap_without_overlap, | ||
| impl.create_lgmap(fine_mesh.topology_dm), |
There was a problem hiding this comment.
This means that this could be moved out of the if-else statement
| impl.create_lgmap(fine_mesh.topology_dm), | |
| impl.create_lgmap(transfer_mesh.topology_dm), |
| return plex_data | ||
|
|
||
|
|
||
| def adaptive_netgen_cells(mesh): |
There was a problem hiding this comment.
Add the new code to the bottom of the file.
| MPI.REPLACE) | ||
|
|
||
|
|
||
| def set_partitioner(dm, parameters): |
There was a problem hiding this comment.
This code can be reused in mesh.py, and that's where it should live.
| }[partitioner_type]) | ||
|
|
||
|
|
||
| def distribute_overlap(dm, parameters): |
There was a problem hiding this comment.
This code can be reused in mesh.py, and that's where it should live.
Description
AI-assisted
Deprecate hierarchy-agnostic
AdaptiveTransferManagerin favour ofTransferManager, now with adaptivity support.