On Monday September 2 2013 10:09:56 Garth N. Wells 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.

How? With a map of maps, where the second is the local dof index?

> >> >   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.

Using mesh.coordinates() was just an example on some data which comes together 
with the mesh. I just high-jacked coordinates to represent some vector field 
aligned with the mesh. Real data often comes aligned with the mesh and we need 
a simple and intuitive way to read such data into a Function. This is basic 
functionality alot of users need for their applications. 

If you intend to include some mesh reordering, I suggest that you also include 
some mapping that brings mesh data to reordered mesh data, and then we need a 
map to bring reordered mesh data to dof ordering. 

> >> > 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.

True. But the two maps, can only be used on data defined on vertices (CG1). 
compute_vertex_values are more general as it works for Functions on a lot more 
FunctionSpaces (CG2, DG0, aso)?

> > 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.

See Martin's answer.

Johan

> 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

Reply via email to