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
_______________________________________________
DOLFIN-dev mailing list
[email protected]
http://www.fenics.org/mailman/listinfo/dolfin-dev
