On 9 Jan 2014, at 08:58, Mikael Mortensen <[email protected]> wrote:
> Hi
>
> Nice to know you're immediately on top of this:-)
>
> One comment is necessary regarding freeing memory by clearing connectivities
> that are no longer needed. The MeshConnectivity._connections storage is an
> std::vector and mesh.clean() calls _connections.clear(). This sets the size
> of the vector to zero, but it does not actually free any memory.
Interesting - I didn’t know this.
> (Just try mesh.clean() after mesh.init() and monitor the memory usage.) To
> actually free memory I have found this solution (from
> www.cplusplus.com/reference/vector/vector/clear/)
>
> diff --git a/dolfin/mesh/MeshConnectivity.cpp
> b/dolfin/mesh/MeshConnectivity.cpp
> index f2ab191..a6ade06 100644
> --- a/dolfin/mesh/MeshConnectivity.cpp
> +++ b/dolfin/mesh/MeshConnectivity.cpp
> @@ -59,8 +59,8 @@ const MeshConnectivity& MeshConnectivity::operator= (const
> MeshConnectivity& con
> //-----------------------------------------------------------------------------
> void MeshConnectivity::clear()
> {
> - _connections.clear();
> - index_to_position.clear();
> + std::vector<unsigned int>().swap(_connections);
> + std::vector<unsigned int>().swap(index_to_position);
> }
> //-----------------------------------------------------------------------------
> void MeshConnectivity::init(std::size_t num_entities, std::size_t
> num_connections)
>
> Any objections if I make this a pull request?
No objection - looks good.
> Furthermore, clear() is used quite extensively so I can't help thinking that
> freeing memory is relevant also for other parts of the code?
>
Yes, we should check that we don’t need it elsewhere.
Garth
> Mikael
>
>
>
> Den Jan 8, 2014 kl. 6:17 PM skrev Anders Logg:
>
>> On Wed, Jan 08, 2014 at 05:00:20PM +0000, Garth N. Wells wrote:
>>> On 2014-01-08 16:48, Anders Logg wrote:
>>>> On Wed, Jan 08, 2014 at 04:41:17PM +0000, Garth N. Wells wrote:
>>>>> On 2014-01-08 16:17, Anders Logg wrote:
>>>>>> On Wed, Jan 08, 2014 at 04:07:35PM +0000, Garth N. Wells wrote:
>>>>>>> On 2014-01-08 15:21, Anders Logg wrote:
>>>>>>>> I would claim that the connectivity is stored very efficiently. The
>>>>>>>> simplest example is D--0 connectivity for a mesh which is for each
>>>>>>>> cell which vertices belong to it. That data is stored as one big array
>>>>>>>> of integers, something like
>>>>>>>>
>>>>>>>> 0 1 2 3 0 1 2 4 ...
>>>>>>>>
>>>>>>>> which means that tetrahedron 0 has vertices 0 1 2 3, tetrahedron 1 has
>>>>>>>> vertices 0 1 2 4 etc.
>>>>>>>>
>>>>>>>> There are D + 1 different kinds of entities in a mesh. For a
>>>>>>>> tetrahedron we have vertex, edge, face, cell and so the total number
>>>>>>>> of connectivities that may be computed is (D + 1)*(D + 1). Not all
>>>>>>>> these are needed, but they can all be computed.
>>>>>>>>
>>>>>>>> For solving Poisson's equation, we first need D--0. To see which
>>>>>>>> connectivities are computed, one can do info(mesh, True) to print the
>>>>>>>> following little table:
>>>>>>>>
>>>>>>>> 0 1 2 3
>>>>>>>> 0 - - - -
>>>>>>>> 1 - - - -
>>>>>>>> 2 - - - -
>>>>>>>> 3 x - - -
>>>>>>>>
>>>>>>>> To set boundary conditions, one needs to create the faces of the mesh
>>>>>>>> (which are not stored a priori). One will then get the following
>>>>>>>> connectivities:
>>>>>>>>
>>>>>>>> 0 1 2 3
>>>>>>>> 0 - - - x
>>>>>>>> 1 - - - -
>>>>>>>> 2 x - - -
>>>>>>>> 3 x - x x
>>>>>>>>
>>>>>>>> Here we see the following connectivities:
>>>>>>>>
>>>>>>>> 3--0 = the vertices of all cells = the cells
>>>>>>>> 0--3 = the cells connected to all vertices
>>>>>>>> 3--3 = the cells connected to all cells (via vertices)
>>>>>>>> 2--0 = the vertices of all faces = the faces
>>>>>>>> 3--2 = the faces of all cells
>>>>>>>>
>>>>>>>> These get computed in order:
>>>>>>>>
>>>>>>>> 0--3 is computed from 3--0 (by "inversion")
>>>>>>>> 3--3 is computed from 3--0 and 0--3
>>>>>>>> 2--0 and 3--2 are computed from 3--3 by a local search
>>>>>>>>
>>>>>>>> 3--3 is needed in order for a cell to communicate with its neighboring
>>>>>>>> cells to decide on how to number the common face (if any).
>>>>>>>>
>>>>>>>
>>>>>>> 3--3 connectivity is *undefined*.
>>>>>>
>>>>>> Yes, in general, but we have *chosen* to define it as 3--0--3.
>>>>>>
>>>>>
>>>>> Which I think we should attempt to remove
>>>>>
>>>>>> The reason for this choice is that we start with 3--0, from which we
>>>>>> can easily compute 3--0--3.
>>>>>>
>>>>>> In other words, we start with cell-vertex data and hence the only way
>>>>>> for two cells to figure out if they share common entities (for example
>>>>>> faces) is to go via the vertices. Unless you have some other brilliant
>>>>>> idea.
>>>>>>
>>>>>
>>>>> The function GraphBuilder::compute_local_dual_graph does this. It
>>>>> loops over cells and builds a hashed map (boost::unordered_map) from
>>>>> a (sorted) vector of the facet vertices to the cell index. If the
>>>>> key (the facet vertices) is already in the map, then we know a cell
>>>>> that the current cell shares a facet with. If the key is not already
>>>>> in the map, then we add it. With appropriate hashing, the
>>>>> unordered_map look-ups are O(1).
>>>>
>>>> Now that's a brilliant idea. Why is this not already the default?
>>>>
>>>
>>> Reluctance to touch the beautifully crafted mesh library? :)
>>
>> :-)
>>
>>> I'll
>>> add it alongside the current code so that we can test it for speed.
>>
>> Great.
>>
>>> It's used in GraphBuilder to construct a dual graph (cell-facet-cell
>>> connections) to feed to a graph partitioner because in GraphBuilder
>>> we don't yet have a Mesh object - just cell connectivity data.
>>>
>>>> Application of boundary conditions can bypass TopologyComputation.cpp
>>>> and build the facets using the dual graph and it would not be in
>>>> conflict with TopologyComputation.cpp (which may still be useful for
>>>> computing other connectitivies).
>>>
>>> The approach in GraphBuilder::compute_local_dual_graph could be used
>>> for other connection types.
>>
>> Definitely. The current (old) approach was implemented based on the
>> belief that mesh-wide hash maps would be very expensive and that
>> the only sane option was local searches.
>>
>> --
>> Anders
>> _______________________________________________
>> fenics mailing list
>> [email protected]
>> http://fenicsproject.org/mailman/listinfo/fenics
>
_______________________________________________
fenics mailing list
[email protected]
http://fenicsproject.org/mailman/listinfo/fenics