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

Fix regression introduced in #2337 for dolfinx.mesh.create_interval #2344

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

francesco-ballarin
Copy link
Member

Hi,
I believe this was introduced yesterday in #2337.

pybind11 wrappers for dolfinx.mesh.create_mesh_partitioner with no input arguments have been removed,
compare
https://github.com/FEniCS/dolfinx/blame/main/python/dolfinx/wrappers/mesh.cpp#L369
to
https://github.com/FEniCS/dolfinx/blame/4a5d20a9577b73052667644611fc014cd17257c1/python/dolfinx/wrappers/mesh.cpp#L380

which on main, at least for my local installation, makes a simple code such as

import dolfinx.mesh
import mpi4py.MPI
dolfinx.mesh.create_interval(mpi4py.MPI.COMM_WORLD, 30, [1, 5])

fail with

Traceback (most recent call last):
  File "test.py", line 3, in <module>
    dolfinx.mesh.create_interval(mpi4py.MPI.COMM_WORLD, 30, [1, 5])
  File "dolfinx/mesh.py", line 324, in create_interval
    partitioner = _cpp.mesh.create_cell_partitioner()
TypeError: create_cell_partitioner(): incompatible function arguments. The following argument types are supported:
    1. (arg0: dolfinx.cpp.mesh.GhostMode) -> Callable[[MPICommWrapper, int, int, dolfinx::graph::AdjacencyList<long>], dolfinx::graph::AdjacencyList<int>]
    2. (part: Callable[[MPICommWrapper, int, dolfinx::graph::AdjacencyList<long>, bool], dolfinx::graph::AdjacencyList<int>]) -> Callable[[MPICommWrapper, int, int, dolfinx::graph::AdjacencyList<long>], dolfinx::graph::AdjacencyList<int>]

I am not sure why CI did not catch this. I cannot even reproduce on dolfinx/dolfinx:nightly docker image.

Copy link
Contributor

@chrisrichardson chrisrichardson left a comment

Choose a reason for hiding this comment

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

I must have missed this one. As you say, it should have been picked up by the CI. Why not?

@chrisrichardson chrisrichardson merged commit f437b34 into main Sep 2, 2022
@chrisrichardson chrisrichardson deleted the fix-2337 branch September 2, 2022 10:21
@francesco-ballarin
Copy link
Member Author

It seems that dolfinx.mesh.create_interval is not used at all in the unit tests. I have added a new commit using it once.

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.

None yet

2 participants