Broken DofMap::dofs()

[Jan Blechta]

The following code demonstrates that DofMap::dofs() is now broken in parallel:

from dolfin import *
mesh = UnitSquareMesh(4, 4)
V = FunctionSpace(mesh, 'Lagrange', 1)
assert MPI.sum(mesh.mpi_comm(), V.dofmap().dofs().size) == V.dim()

It works fine with 1.4.0. Function is nowhere used in DOLFIN grep -r "[^_]dofs()" dolfin/ site-packages/ test/ demo/.

[Jan Blechta]

Fix in pull request #150.

[Prof Garth Wells]

This doesn't indicate that the code is broken. Ambiguity in the doc string should be resolved.

I'm happy to revisit this question in the coming weeks once we have merged ghosted meshes, which will provide guidance on how functions like this should look going forward. We may want to deprecate the function.

[Jan Blechta]

The code is indeed broken. You're checking local DOF index against global ownership range. It may happen on some meshes and partitioning that empty vector is returned on some processes which obviously wrong.

Consider before switching whether you want to break user code. DofMap::dofs() may be, for example, used for setting ISes for fieldsplit, like in your magma dynamics code. It must definitely got broken. I don't know currently how to do this now using local DOFs but I expect that it is possible.

[Jan Blechta]

Check line 382 - checking local DOF against global range.

[Jan Blechta]

@garth-wells The suggested fix makes your magma dynamics code again working with small workaround circumventing other DOLFIN issue I will report later (EDIT: #344).

Consider, please, this change to DofMap::dofs(). It really makes fieldsplit working again. I understand that you're working on some bigger changes with @chris_richardson currently, nevertheless the code in DofMap::dofs() is really broken regardless of what it is intended to do.

[Prof Garth Wells]

Fixed in f77a3aa

[Prof Garth Wells]

Removing milestone: 1.5 (automated comment)