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

Permutation connection #445

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jonathan-macart
Copy link

Added:

  1. get_reordering_by_pymetis: Compute mesh element reorderings (permutations) via Metis's nested dissection algorithm.
  2. make_element_permutation_connection: First attempt at generating a connection that permutes the elements of an existing mesh, say from_mesh, to those of a new mesh, to_mesh, using the ND-generated permutation. The goal is to be able to pass to_mesh to a CFD solver. I do not need the mesh nodes to be reordered. My understanding is that all_elements_perm should contain the permuted element tags of from_mesh, though this could be completely incorrect.

Currently, (2) returns a discretization connection containing two identical discretizations/meshes. A few thoughts/questions:

  • Is constructing to_discr = from_discr.copy() a problem? My understanding is that this creates an exact replica of from_discr.mesh, which is not the goal. to_discr.mesh should contain the same nodes and elements of from_discr.mesh, but with the mesh ordering permuted using perm.
  • Is a discretization connection a mapping between two "immutable" meshes? Or is to_discr.mesh actually changed upon creation of the connection?
  • If the answer to the last question is "no," how do I construct a copy of from_discr.mesh with the elements permuted?

Any advice would be appreciated! Thank you!

@inducer
Copy link
Owner

inducer commented Dec 6, 2024

Side note: please disregard the RUF052 (astral-sh/ruff#14796) and pylint linter failures (pylint-dev/pylint#10000 (comment)).

@@ -0,0 +1,90 @@
__copyright__ = "Copyright (C) 2014 Andreas Kloeckner"
Copy link
Owner

Choose a reason for hiding this comment

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

Please fix the copyright notice.

Comment on lines +48 to +49
from meshmode.distributed import get_reordering_by_pymetis
perm, iperm = get_reordering_by_pymetis(from_discr.mesh)
Copy link
Owner

Choose a reason for hiding this comment

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

Could you make the permutation a parameter to this function, rather than unconditionally obtaining it from pymetis?


# Reorder the local mesh using Metis
from meshmode.distributed import get_reordering_by_pymetis
perm, iperm = get_reordering_by_pymetis(from_discr.mesh)
Copy link
Owner

Choose a reason for hiding this comment

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

Will perm, iperm match the return value of get_reordering_by_pymetis? It seems that returns only a single array?


# {{{ permute mesh elements

def make_element_permutation_connection(actx, from_discr):
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
def make_element_permutation_connection(actx, from_discr):
def make_element_permutation_connection(actx: ArrayContext, from_discr: Discretization) -> DirectDiscretizationConnection:

@@ -445,6 +445,49 @@ def get_partition_by_pymetis(mesh, num_parts, *, connectivity="facial", **kwargs
return np.array(p)


# FIXME: Move somewhere else, since it's not strictly limited to distributed?
def get_reordering_by_pymetis(mesh, *, connectivity="facial", **kwargs):
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
def get_reordering_by_pymetis(mesh, *, connectivity="facial", **kwargs):
def get_reordering_by_pymetis(mesh: meshmode.mesh.Mesh, *, connectivity: Literal["facial"] | Literal["nodal"], **kwargs) -> np.ndarray:


# {{{ permute mesh elements

def make_element_permutation_connection(actx, from_discr):
Copy link
Owner

Choose a reason for hiding this comment

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

From @jonathan-macart's email, my understanding was that the objective of this is a mesh with the elements reordered. This connection instead shuffles DOF data between elements, while leaving the discretization in place.

To achieve the goal I mention above, you would need to make a permuted mesh, then make a discretization based on that.

Comment on lines +74 to +75
from_element_indices=all_elements,
to_element_indices=all_elements_perm, ## CHK??
Copy link
Owner

Choose a reason for hiding this comment

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

This mishandles meshes/discretizations with multiple element groups (e.g. a mesh that's part-hex-part-tet), in that it is silently broken (by using out-of-bounds indices) in that case. It's fine to assume single-group only, but then the code should error if presented with multiple groups.

@@ -445,6 +445,49 @@ def get_partition_by_pymetis(mesh, num_parts, *, connectivity="facial", **kwargs
return np.array(p)


# FIXME: Move somewhere else, since it's not strictly limited to distributed?
def get_reordering_by_pymetis(mesh, *, connectivity="facial", **kwargs):
Copy link
Owner

Choose a reason for hiding this comment

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

I think an argument could be made that the connectivity-getter could be factored into a separate function, since it seems it's duplicated from get_partition_by_pymetis?

Copy link
Owner

Choose a reason for hiding this comment

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

Please add tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants