On Mon, 2 Sep 2013 10:09:56 +0100 "Garth N. Wells" <[email protected]> wrote:
> On 2 September 2013 09:58, Johan Hake <[email protected]> wrote: > > On Monday September 2 2013 09:42:49 Garth N. Wells wrote: > >> On 2 September 2013 09:30, Johan Hake <[email protected]> wrote: > >> > Seems like updating the doc string wont help as enough people > >> > have tried to > >> > use the vertex_to_dof_map and failed. > >> > > >> > I agree that the left to right reading does not apply to the > >> > example Garth presented. If that is the expected behavior, and I > >> > guess it is given the comments in this treahd, we should just > >> > rename the methods. That would generalize the methods and > >> > probably fit better into the general interface of DofMap. > >> > > >> > However that would limit the scope of the map and remove one > >> > important motivation for adding the map in the first place, > >> > namely to turn general > >> > >> > vector function values ordered as: > >> How can renaming limit scope? The functionality remains the same. > > > > Because it is not enough to just rename it. We also need to remove > > the functionality for vector function spaces. Your example does not > > make sense if you change: > > > > V = FunctionSpace(mesh, "Lagrange", 1) > > > > to > > > > V = VectorFunctionSpace(mesh, "Lagrange", 1) > > > > My example was deliberately simple. The functionality can be retained. > > >> > vertex_index*dofs_per_vertex+local_dof > >> > > >> > to an array which could be feed directly into a vector of a > >> > Function in a VectorFunctionSpace (or similar mixed CG1 function > >> > spaces). The present functionality also works for parallel runs, > >> > as seen by the following example: > >> > > >> > mpirunt -np 2 python vertex_to_dofs.py > >> > > >> > # vertex_to_dofs.py > >> > > >> > from dolfin import * > >> > import numpy as np > >> > > >> > mesh = UnitSquareMesh(20,20) > >> > V = VectorFunctionSpace(mesh, "CG", 1) > >> > u = Function(V) > >> > vertex_to_dof_map = V.dofmap().vertex_to_dof_map(mesh) > >> > > >> > data = np.reshape(mesh.coordinates()[:], (mesh.num_vertices()*2)) > >> > >> This is problematic - it makes an assumption of the ordering in > >> mesh.coordinates(). > > > > The only assumption is that you have some data (possible vector or > > tensor data) which are ordered based on the mesh (vertices). > > > >> I have seen that a good re-ordering of mesh data > >> can give up to a 50% speed up for assembly, and which will be > >> added in the future. We should not be exposing low-level storage. > > > > Not sure what you mean. This has nothing to do with assemble. Only > > transferring vertex based data into a Function. > > > > Exposing low level storage (e.g. (mesh.coordinates()), violates data > hiding, which then can affect all parts of a code. If the mesh data > ordering is changed, say to make assembler faster, your example code > will likely break. > > >> > u.vector().set_local(data[vertex_to_dof_map]) > >> > plot(u, interactive=True) > >> > >> Why not just use Function::compute_vertex_values(...) (plus any > >> necessary generalisation)? > > > > The comparison with compute_vertex_values is appropriate. It was > > raised when we discussed the inclusion of the map in the first > > place. However the (present) vertex_to_dof_map give the mapping > > from vertex based data to a Function, where compute_vertex_values > > does the opposite. > > Yes, but two functions were added to GenericDofMap. One seems to > duplicate existing functionality. I can't see how `DofMap::foo_to_bar_map` maps duplicate `Function::compute_vertex_values`. They compute different things. Moreover, `Function::compute_vertex_values` is based on a robust approach which works for any function space and even on non-matching mesh. The two concerned maps just work for vertex-based function spaces, right? Jan > > > The map is also just > > computed once and can therefore be reused by the user if that is > > needed. > > > > I don't see the benefit if one can use > Function::compute_vertex_values. > > Garth > > > > Johan > > > >> Garth > >> > >> > Johan > >> > > >> > On Saturday August 31 2013 10:20:21 Simone Pezzuto wrote: > >> >> Hi, > >> >> > >> >> I'm familiar with these two maps since I use them for > >> >> a gradient > >> >> > >> >> recovery technique. > >> >> > >> >> I can assure you that first time I used vertex_to_dof_map I was > >> >> a bit confused, > >> >> since the convention should be left to right (as Garth pointed > >> >> out). > >> >> > >> >> Example: eps2pdf fig.eps ---> fig.pdf > >> >> > >> >> vertex2dof vertex_id --> dof_id > >> >> dof2vertex dof_id --> vertex_id > >> >> > >> >> So at the moment is really confusing. Maybe we can introduce new > >> >> functions > >> >> {vertex2dof,dof2vertex}_map > >> >> (no name collision) and deprecate the old one, so the user is > >> >> aware of the > >> >> change but its code doesn't brake. > >> >> > >> >> Simone > >> >> > >> >> 2013/8/31 Jan Blechta <[email protected]> > >> >> > >> >> > On Fri, 30 Aug 2013 23:47:35 +0100 > >> >> > > >> >> > "Garth N. Wells" <[email protected]> wrote: > >> >> > > On 30 August 2013 23:37, Johan Hake <[email protected]> > >> >> > > wrote: > >> >> > > > On Friday August 30 2013 23:19:09 Garth N. Wells wrote: > >> >> > > >> On 30 August 2013 22:50, Johan Hake > >> >> > > >> <[email protected]> wrote: > >> >> > > >> > On Friday August 30 2013 15:47:28 Garth N. Wells wrote: > >> >> > > >> >> The functions GenericDofmap::vertex_to_dof_map and > >> >> > > >> >> GenericDofMap::dof_to_vertex_map are not properly > >> >> > > >> >> documented (the doc string is the same for both), and > >> >> > > >> >> I think that they are back to front. The docstring in > >> >> > > >> >> DofMap has inconsistencies. I would expect that > >> >> > > >> >> > >> >> > > >> >> map0 = GenericDofmap::vertex_to_dof_map(...) > >> >> > > >> >> > >> >> > > >> >> would mean a map from vertex to dof, i.e. > >> >> > > >> >> > >> >> > > >> >> map0[vertex_index] -> dof index > >> >> > > >> >> > >> >> > > >> >> and that > >> >> > > >> >> > >> >> > > >> >> map1 = GenericDofmap::dof_to_vertex_map(...) > >> >> > > >> >> > >> >> > > >> >> would mean a map from dof index to > >> >> > > >> >> > >> >> > > >> >> map1[dof_index] -> vertex index > >> >> > > >> >> > >> >> > > >> >> Tests (see below code) and the return types also > >> >> > > >> >> indicate that things are back to front. Can someone > >> >> > > >> >> clarify the situation? > >> >> > > >> > > >> >> > > >> > The map was introduced to help a user map vertex based > >> >> > > >> > data onto a Function.> > >> >> > > >> > > >> >> > > >> > from dolfin import * > >> >> > > >> > import numpy as np > >> >> > > >> > > >> >> > > >> > mesh = UnitSquareMesh(20,20) > >> >> > > >> > V = VectorFunctionSpace(mesh, "CG", 1) > >> >> > > >> > u = Function(V) > >> >> > > >> > vertex_to_dof_map = > >> >> > > >> > V.dofmap().vertex_to_dof_map(mesh) > >> >> > > >> > > >> >> > > >> > data = np.reshape(mesh.coordinates()[:], > >> >> > > >> > > >> >> > > >> > (mesh.num_vertices()*2)) u.vector()[:] = > >> >> > > >> > data[vertex_to_dof_map] > >> >> > > >> > > >> >> > > >> > plot(u, interactive=True) > >> >> > > >> > > >> >> > > >> > The size of the data array should be: > >> >> > > >> > mesh.num_vertices()*u.value_size() > >> >> > > >> > > >> >> > > >> > The documentation should be improved, and not least > >> >> > > >> > properly mapped from C++ to Python. > >> >> > > >> > > >> >> > > >> > The name refer to the mapping that turn vertex based > >> >> > > >> > data to dof based and reads quite well when used as > >> >> > > >> > above. I can see that the word map can be missleading. > >> >> > > >> > It is not a "map" data structure. It is an index set > >> >> > > >> > that "maps values". > >> >> > > >> > > >> >> > > >> > Still confused? > >> >> > > >> > >> >> > > >> I'm not confused. It's clear that the function names are > >> >> > > >> back-to-front. It doesn't matter what they were included > >> >> > > >> for - they > >> >> > > >> are members of GenericDofMap and must make sense in that > >> >> > > >> context. > >> >> > > >> > >> >> > > >> Since reading from left to right is a well established > >> >> > > >> convention, I propose that (a) the function names be > >> >> > > >> fixed by reversing them; and (b) the doc strings be > >> >> > > >> fixed. > >> >> > > > > >> >> > > > Agree on (b). I am not fully convinced by (a). > >> >> > > > > >> >> > > > I am not sure what your example tries to show. You are > >> >> > > > not using the mapping the intended way and I am therefore > >> >> > > > confused about the whole back-to-front, front-to-back > >> >> > > > discussion. > >> >> > > > >> >> > > Just read the function names aloud from left to right - > >> >> > > 'vertex_to_dof_map' should be a 'vertex to dof map', i.e. a > >> >> > > map from a > >> >> > > vertex *to* a dof. > >> >> > > >> >> > Just read from left to right - 'vertex_to_dof_map' stands for > >> >> > a map which turns a vertex map into a dof map (when used as a > >> >> > right composition). > >> >> > > >> >> > Yes, I was confused at first when I saw this and agree with > >> >> > Garth it should be 'left to right'. But does it worth > >> >> > switching it? Is the whole concept of indexing by > >> >> > > >> >> > vertex_index*dofs_per_vertex+local_dof > >> >> > > >> >> > sustainable? Or should it be replaced by some more robust > >> >> > types which would handle non-injective map (and its > >> >> > inversion)? > >> >> > > >> >> > There were some user codes using these functions as seen in > >> >> > discussions. > >> >> > > >> >> > Jan > >> >> > > >> >> > > Garth > >> >> > > > >> >> > > > Johan > >> >> > > > > >> >> > > >> Garth > >> >> > > >> > >> >> > > >> > Johan > >> >> > > >> > > >> >> > > >> >> Garth > >> >> > > >> >> > >> >> > > >> >> > >> >> > > >> >> > >> >> > > >> >> from dolfin import * > >> >> > > >> >> > >> >> > > >> >> mesh = UnitSquareMesh(4, 4) > >> >> > > >> >> V = FunctionSpace(mesh, "Lagrange", 1) > >> >> > > >> >> > >> >> > > >> >> dof_to_vertex = V.dofmap().dof_to_vertex_map(mesh) > >> >> > > >> >> vertex_to_dof = V.dofmap().vertex_to_dof_map(mesh) > >> >> > > >> >> > >> >> > > >> >> for c in cells(mesh): > >> >> > > >> >> print "Cell index:", c.index() > >> >> > > >> >> > >> >> > > >> >> # Get cell dofs > >> >> > > >> >> dofs = V.dofmap().cell_dofs(c.index()) > >> >> > > >> >> print " Cell dofs:", dofs > >> >> > > >> >> > >> >> > > >> >> # Get vertices from cell > >> >> > > >> >> cell_vertices0 = sorted([v.index() for v in > >> >> > > >> >> vertices(c)]) print " Cell vertex indices (from > >> >> > > >> >> cell):", cell_vertices0 > >> >> > > >> >> > >> >> > > >> >> # Get vertices from dof_to_vertex > >> >> > > >> >> cell_vertices1 = sorted([dof_to_vertex[dof] for > >> >> > > >> >> dof in > >> >> > > >> >> > >> >> > > >> >> dofs]) print " Cell vertex indices (from > >> >> > > >> >> dof_to_vertex_map):", > >> >> > > >> >> > >> >> > > >> >> cell_vertices1 > >> >> > > >> >> > >> >> > > >> >> # Get vertices from vertex_to_dof_map > >> >> > > >> >> cell_vertices2 = sorted([vertex_to_dof[dof] for > >> >> > > >> >> dof in > >> >> > > >> >> > >> >> > > >> >> dofs]) print " Cell vertex indices (from > >> >> > > >> >> vertex_to_dof_map):", > >> >> > > >> >> > >> >> > > >> >> cell_vertices2 > >> >> > > >> >> > >> >> > > >> >> _______________________________________________ > >> >> > > >> >> fenics mailing list > >> >> > > >> >> [email protected] > >> >> > > >> >> http://fenicsproject.org/mailman/listinfo/fenics > >> >> > > > >> >> > > _______________________________________________ > >> >> > > fenics mailing list > >> >> > > [email protected] > >> >> > > http://fenicsproject.org/mailman/listinfo/fenics > >> >> > > >> >> > _______________________________________________ > >> >> > fenics mailing list > >> >> > [email protected] > >> >> > http://fenicsproject.org/mailman/listinfo/fenics > >> > > >> > _______________________________________________ > >> > fenics mailing list > >> > [email protected] > >> > http://fenicsproject.org/mailman/listinfo/fenics > >> > >> _______________________________________________ > >> fenics mailing list > >> [email protected] > >> http://fenicsproject.org/mailman/listinfo/fenics > _______________________________________________ > fenics mailing list > [email protected] > http://fenicsproject.org/mailman/listinfo/fenics _______________________________________________ fenics mailing list [email protected] http://fenicsproject.org/mailman/listinfo/fenics
