dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

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?

-- 
Anders

Attachment: signature.asc
Description: Digital signature