← Back to team overview

dolfin team mailing list archive

Re: More comments on PXMLMesh.cpp

 

On Mon, Sep 22, 2008 at 05:03:59PM +0200, Niclas Jansson wrote:
> Anders Logg wrote:
> > I'm slowly working my way through PXMLMesh.cpp and I need some
> > assistance (from Niclas).
> > 
> > It all looks very good and I'm sure it works perfectly, but some work
> > is needed to make it possible to read/understand:
> > 
> > 1. PXMLMesh::closeMesh really needs some commenting. It's a large
> > chunk of nontrivial and quite complex code so a whole lot of
> > commenting is needed.
> 
> I'm working on it.

ok, nice.

> > 2. Why is the mesh editor closed in PXMLMesh::readCells and then
> > opened again later in PXMLMesh::closeMesh?
> > 
> > If we can't write to the mesh until everything is read in, then maybe
> > we should wait with initializing anything until closeMesh() and at
> > that point start editing the mesh?
> > 
> 
> It's a (ugly) workaround due to the static nature of the MeshEditor 
> (number of vertices must be specified a priori). It could of course be 
> fixed by moving the geometric partitioner inside the parser, and only 
> opening the editor inside closeMesh().
> 
> However the nice solution would be to make the MeshEditor more dynamic. 
> That would also make life easier when implementing a better refinement 
> algorithm (for example, the recursive longest edge bisection (Rivara) 
> from unicorn)

I agree it would be better to make MeshEditor dynamic. The problem is
that this is not always needed. Perhaps we should add a new class
DynamicMeshEditor that can be used when one does not know the number
of vertices and cells a priori.

It could be very simple, just storing the dynamic data in suitable STL
containers and then calling MeshEditor in close().

> > 3. I don't understand the logic in readTriangle/readTetrahedron.
> > What does the following code do?
> > 
> >   if (!(is_local(v2) || is_local(v1) || is_local(v0)) || !is_local(v0))
> >     return;
> > 
> >   used_vertex.insert(v0);
> >   if (is_local(v1))
> >     used_vertex.insert(v1);
> >   if (is_local(v2))
> >     used_vertex.insert(v2);
> > 
> >   if (!(is_local(v1) && is_local(v2) && is_local(v0)))
> >   {
> >     if (!is_local(v1))
> >       shared_vertex.insert(v1);
> >     if (!is_local(v2))
> >       shared_vertex.insert(v2);
> >   }
> > 
> 
> The idea is to assign the triangle/tetrahedron to the processor who owns 
>   vertex v0. And, yes the logical could probably be more clearer.

ok, I see.

It seems that

  !(is_local(v2) || is_local(v1) || is_local(v0)) || !is_local(v0)

is equivalent to

  (!is_local(v2) && !is_local(v1) && !is_local(v0)) || !is_local(v0)

which is equivalent to

  !is_local(v0)

Is this correct, and is this what you have in mind?

> The used_vertex part marks which of the locally stored vertices that a 
> processor have used. Since some of them are going to be unused, they 
> "must" be moved of the processor in a later stage.
> 
> The shared_vertex parts marks which vertices not present at the 
> processor a cell is using.
> 
> > I've added the function is_local to make it more readable but it's
> > still not clear to me what happens.
> > 
> 
> While parsing the cells, a processor must decide if it have the parsed 
> global vertex number.
> 
> Instead of search trough the mesh function of global numbers, all 
> numbers are placed in a std::set (after the geometric partitioner), so 
> that lookups could be made in a reasonable amount of time ( O(log n) if 
> I remember correctly).
> 
> The best solution would of course be to implemented some kind of hash table.
>
> > 4. The above logic seems to be missing in readInterval.
> 
> Yes, I simply forgot to add it.

ok.

-- 
Anders

Attachment: signature.asc
Description: Digital signature


Follow ups

References