Mesh constructors no longer take in petsc4py Comm objects

[Patrick Farrell]

The following code used to work before the removal of SWIG:

from dolfin import *
from petsc4py import PETSc
from mpi4py import MPI

mcomm = MPI.COMM_WORLD
pcomm = PETSc.Comm(mcomm)
mesh = UnitIntervalMesh(pcomm, 10)

After the removal of SWIG, it now yields

Traceback (most recent call last):
  File "demo.py", line 7, in <module>
    mesh = UnitIntervalMesh(pcomm, 10)
TypeError: __init__(): incompatible constructor arguments. The following argument types are supported:
    1. dolfin.cpp.generation.UnitIntervalMesh(arg0: int)
    2. dolfin.cpp.generation.UnitIntervalMesh(arg0: MPICommWrapper, arg1: int)

Invoked with: <petsc4py.PETSc.Comm object at 0x7f7c901ae188>, 10

[Prof Garth Wells]

Use pcomm.tompi4py() to turn a petsc4py comm into a mpi4py comm.

We won't be supporting petsc4py comms. Use mpi4py.

[Patrick Farrell]

I think wontfix is a more appropriate status.

Why not add the code to call pcomm.tompi4py() in the new Python bindings, though? It would mean one less unnecessary API change for downstream users. Isn't it easier to do it once centrally than in N different codes?

[Prof Garth Wells]

Because the design decision is to support mpi4py.

[Prof Garth Wells]

To add, the previous handling of MPI comms in the Python layer was deeply flawed, and most likely dictated by the limitations of SWIG. The 'right way' to wrap a comm at the Python level is as a mpi4py object, and this is now the case. It does not make sense that a Mesh, which has nothing to do with PETSc, should accept a petsc4py comm and require petsc4py to be installed.

[Patrick Farrell]

I agree with everything you said. In particular, the mpi4py comm is the "natural" wrapping of an MPI_Comm and should be the main object accepted and used by mesh constructors. It's nicer this way. I also agree that things depend unnecessarily on petsc4py.

However, that doesn't mean things couldn't work nicely with petsc4py if it happens to be there. My thinking was that an additional mesh constructor could be supplied that takes in a petsc4py comm, since (i) they're also pretty common (e.g. from any object in PETSc) (ii) trivial to convert to mpi4py comms (iii) that's what the constructors used to take in. I thought it might be possible to do that once centrally in one place (e.g. in the MPICommWrapper stuff) rather than in N user codes. Unfortunately I don't understand pybind11 well enough to understand the relevant type_caster to guess how straightforward or complex implementing this suggestion would be.

I do agree it's not that important, though.