← Back to team overview

dolfin team mailing list archive

Re: More comments on PXMLMesh.cpp

 

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


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




Follow ups

References