Skip to content

Introduce create_identity_partitioner for refined mesh partitioning #3661

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

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

Conversation

schnellerhase
Copy link
Contributor

@schnellerhase schnellerhase commented Mar 11, 2025

Introduces create_identity_partitioner which facilitates the exchange of ghost cells of a refined mesh whilst maintaining the partition of the coarse mesh. This introduces the necessity of a new sentinel variable redistribute for the python refine functionality to distinguish between the C++ cases of nullptr and nullopt correctly.

This fixes (partially) #3443.

Additionally, this changes

  1. move compute_destination_ranks from anonymous namespace to dolfinx::graph,
  2. python dolfinx.mesh.refine 's partitioner default argument value was not aligning with cpp layer, fixed,
  3. default argument for refine's option arg changed from Option::none to Option::parent_cell,
  4. partitioner argument of refine is now std::optional, allows for handling of case of not provided (which is different from providing nullptr as callable).

@schnellerhase
Copy link
Contributor Author

Verification code:

import febug
import pyvista
import dolfinx
import dolfinx.mesh
from mpi4py import MPI

mesh = dolfinx.mesh.create_unit_square(MPI.COMM_WORLD, 4, 4)
mesh.topology.create_entities(2 - 1)
mesh.topology.create_connectivity(2 - 1, 2)
mesh.topology.create_entities(1)
[refined_mesh, _, _] = dolfinx.mesh.refine(
    mesh, option=dolfinx.mesh.RefinementOption.parent_cell
)
subplotter = pyvista.Plotter(shape=(2, 1))

subplotter.subplot(0, 0)
febug.plot_mesh(mesh, plotter=subplotter)

subplotter.subplot(1, 0)
febug.plot_mesh(refined_mesh, plotter=subplotter)
# subplotter.show()
subplotter.save_graphic(f"out_{MPI.COMM_WORLD.rank}.pdf")

Output ($np=2$):
out_0.pdf
out_1.pdf

Output ($np=3$):
out_0.pdf
out_1.pdf
out_2.pdf

@schnellerhase
Copy link
Contributor Author

schnellerhase commented Mar 11, 2025

Tests fail due to refinement routine now not being able to handle the case of no partitioner and no parent_cell flag being set anymore. This originates from the need to have parent_cell computed before the partitioner may be constructued. Should we disallow this combination?

Also, I have tested the implementation with the above code snippet - not sure how to extend that to a reasonable unit test though.

@schnellerhase schnellerhase marked this pull request as ready for review March 11, 2025 21:39
@schnellerhase schnellerhase changed the title Fix refined mesh partitioning for ghosted meshes Introduce create_identity_partitioner for refined mesh partitioning Mar 12, 2025
@schnellerhase
Copy link
Contributor Author

schnellerhase commented Mar 19, 2025

I have a problem with the std::optional here in the Python layer. The export of the partitioner callback now has signature std::optional<CellPartitionFunction> (to allow for handling the different cases of nullptr and std::nullopt). Passing None results in a std::nullopt and no longer the nullptr as before . However I am not aware of another way to produce a nullptr in the C++ layer other than by passing None.

It comes down to: how can a std::optional<CellPartitionFunction> = nullptr case be produced form the python caller side?

@schnellerhase
Copy link
Contributor Author

Exporting explicitly the empty partitioner case as

  m.def("empty_partitioner",
        [&]
        {
          return std::make_optional<
              dolfinx_wrappers::part::impl::PythonCellPartitionFunction>(
              nullptr);
        });

also does not work, since this gets auto transformed into a None, i.e. the following fails

def test_empty_partitioner():
    assert empty_partitioner() is not None

@jhale
Copy link
Member

jhale commented Mar 26, 2025

What about using std::variant to explicitly deal with the three options (something, nullptr, nullopt)?

@schnellerhase
Copy link
Contributor Author

I believe that would suffer from the same problem of None being used in Python both for a nullptr and a nullopt. So two of the variants will be None in the Python layer making it again not possible to separate them from one another.

@jhale
Copy link
Member

jhale commented Mar 26, 2025

Right - then I can't think of anyway of doing it except introducing an explicit sentinel type for this purpose.

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