dolfin team mailing list archive

[Niclas Jansson <njansson@xxxxxx>]

Anders Logg wrote:
> On Fri, Sep 26, 2008 at 05:35:56PM +0200, Niclas Jansson wrote:
> > Anders Logg wrote:
> > > On Fri, Sep 26, 2008 at 11:35:38AM +0200, Niclas Jansson wrote:
> > > > Anders Logg wrote:
> > > > > On Mon, Sep 22, 2008 at 06:17:49PM +0200, Anders Logg wrote:
> > > > > > On Mon, Sep 22, 2008 at 06:13:34PM +0200, Niclas Jansson wrote:
> > > > > > > Anders Logg wrote:
> > > > > > > > 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?
> > > > > > > It looks correct to me. Seems like I should refresh my logic, that was a
> > > > > > > really simple reduction :)
> > > > > > >
> > > > > > > Niclas
> > > > > > It wasn't very obvious that it could be so much simplified, but it
> > > > > > should help to clear up the logic.
> > > > > >
> > > > > > I'll wait for you to do some more work on PXMLMesh.cpp before I
> > > > > > continue reading it.
> > > > > >
> > > > > > If possible, I'll add DynamicMeshEditor later tonight so you can use
> > > > > > that to simplify PXMLMesh.cpp further.
> > > > > I've added DynamicMeshEditor. Try to use this in PXMLMesh.cpp. It may
> > > > > simplify it a bit.
> > > > Commenting and bug fixes in PXMLMesh.
> > > Thanks, it looks much better! I'll make a new attempt to get through
> > > the code (after the suggested changes below).
> > >
> > > > I couldn't use DynamicMeshEditor to get rid of the opening and closing
> > > > of the mesh editor. The problem is that each processor parse (and
> > > > receive from partitioning) vertices which are not used in the local
> > > > cells. These must be removed before the final mesh is returned, and this
> > > > is not supported by the editor.
> > > >
> > > > I think the easiest solution to this problem is to move the geometric
> > > > partitioner inside the XML parser. Since it's produces rather bad
> > > > partitions I don't see why anyone would like to use it in an application.
> > > That sounds good, especially since you now call the partitionGeom()
> > > before the mesh is even a mesh (it has only vertices).
> > >
> > > Can you make an attempt to do the geometric partitioning inside
> > > PXMLMesh?
> > >
> > > I also suggest that you don't open the mesh editor until you get to
> > > the closemMesh() function. Just store the vertices in an Array<>
> > > (which is the same as an std::vector). You don't call editor.addCell()
> > > until closeMesh() so it's not really a Mesh before then anyway.
> > >
> > > It also fits well with how you treat the cells. You already have a
> > > cell_buffer. You could store a vertex_buffer similarly.
> > >
> > > So it would be something like this:
> > >
> > > 1. Read vertex coordinates into a simple Array (or similar).
> > >
> > > 2. Partition the vertices.
> > >
> > > 3. Build the Mesh in closeMesh() using MeshEditor.
> > >
> > > Would that work?
> > I think so. I'll try fix this and insert the geometric partitioner as 
> > soon as possible.
> >
> > Niclas
>
> Any progress on the patch?

No. I'll probably have time to fix it this week.

Niclas