Naming of `DofMap::{vertex_to_dof_map,dof_map_to_vertex}` should be changed

[Johan Hake]

As pointed out in this thread the naming of DofMap::{vertex_to_dof_map,dof_map_to_vertex} is right to left, when it should be left to right.

The documentation of the two methods should also be improved.

[Johan Hake]

I suggest we just rename the methods and make a note on the list and in the ChangeLog. Users then need to update code that uses these mappings.

Other more permanent fixes for assigning mesh based values to Function discussed in the thread above need its own issue/feature request.

[Jan Blechta]

There was also suggestion to create new dof2vertex_map and vertex2dof_map, keep original ones but issue a message with deprecation and remove in a future (say 1.3.0 or 1.4.0 release).

[Martin Sandve Alnæs]

But should these really be members in DofMap? I can think of a large amount of variations of "get collections of dofs with properties x,y,z" for different contexts and elements, maybe that should be in a collection of stand-alone functions instead.

[Prof Garth Wells]

I agree that these function should probably not be members of DofMap and be free functions instead. It will then also not contributed to the bloat of DofMap.

Names of the form dof2vertex_map (with the numerall '2') are inconsistent with the function naming convection in DOLFIN.

[Johan Hake]

Free functions sounds very attractive. As Martin says there is a number of useful free functions that could be added. Should we keep a deprecated version in DofMap? With a free function that is very easy.

Where to put these then? Should we add a subdirectory called utils to collect misc such functions?

[Prof Garth Wells]

I would suggest still keeping the functions 'close to home'. Maybe in files dolfin/fem/fem_utils.h/.cpp.

For the old functions, they could print either a deprecation message or throw an error. The problem with deprecation is that removing them later tends to be forgotten.

[Jan Blechta]

You can keep the issue open, convert it to task: remove deprecated DofMap::foo_to_bar_map at release quux.

[Johan Hake]

Fixed by:

21883e5fdd912b4591a86e6057594e0b899ceb08

[Jan Blechta]

There is missing corresponding update in ale/HarmonicSmoothing.cpp.

Testing HarmonicSmoothing and ALE operations
------------------------------------------------
Process 0: Number of global vertices: 121
Process 0: Number of global cells: 200

Testing HarmonicSmoothing::move(Mesh& mesh, const BoundaryMesh& new_boundary)
Process 2: *** -------------------------------------------------------------------------
*** Warning: dof_to_vertex_map has been deprecated in DOLFIN version 1.3.0.
*** DofMap::dof_to_vertex_map has been replaced by the free function vertex_to_dof_map.
*** -------------------------------------------------------------------------

Process 0: *** -------------------------------------------------------------------------
*** Warning: dof_to_vertex_map has been deprecated in DOLFIN version 1.3.0.
*** DofMap::dof_to_vertex_map has been replaced by the free function vertex_to_dof_map.
*** -------------------------------------------------------------------------

Process 1: *** -------------------------------------------------------------------------
*** Warning: dof_to_vertex_map has been deprecated in DOLFIN version 1.3.0.
*** DofMap::dof_to_vertex_map has been replaced by the free function vertex_to_dof_map.
*** -------------------------------------------------------------------------

[Jan Blechta]

Missing update in dolfin/ale/HarmonicSmoothing.cpp.

[Johan Hake]

@blechta isn't this an easy fix? couldn't we just change the name of the used method to the updated name?

V->dofmap()->dof_to_vertex_map(mesh) --> vertex_to_dof_map(V)

[Jan Blechta]

@johanhake I guess so. There's test/unit/ale/python/HarmonicSmoothing.py.

[Johan Hake]

Should be fixed in: 61706e8d55e4a0603e594cf5a6f8e9b074a94714

[Prof Garth Wells]

Removing milestone: 1.4 (automated comment)