[Libmesh-devel] Re: [Libmesh-users] GMV_IO

[Benjamin S. K. <be...@cf...>]

You raise some good points...  I have responded to them below.

Martin L=FCthi wrote:
> Hi all
>=20
> About a week ago I promised to send a patch for the correct export of
> QUAD8 elements to GMV. Now I was looking into doing that, and noted
> some problems / inconsistencies:
>=20
> o is mesh_gmv_support.C obsolete? There, the elements are handled
>   differently than in gmv_IO (no calls to tecplot_connectivity)

Pretty much, but it contains one useful method at the moment.  It allows=20
you to write discontinuous data in such a way that you can actually view=20
its actual discontinuities.  Really, this feature needs to get migrated=20
into the MeshIO class, I just haven't done it.


> o should there be a gmv_connectivity function, which delegates to
>   tecplot_connectivity if there is a special case?
>   Or would a generic get_connectivity(se, type) make more sense?
>   (where type is one of ["gmv", "tecplot", "diva", "vtk"])

See below.

> o There seems to be quite some code duplication, especially in the
>   tecplot_connectivity and vtk_connectivity methods of the different
>   geom/cell_*. It seems that the only difference is that gmv needs a
>   1 based node numbering.

There is a lot of duplication.  In the early days of the lib we were=20
only concerned with a few post-processing tools, since we only had=20
access to a few.  So, the current system evolved.  Now it is showing its=20
age...

I definitely agree that tecplot_conn, vtk_conn, gmv_conn...  is not the=20
right way to go.  It clutters the Elem class and provides a lot of=20
duplication.

One solution is what you suggest, but since it will get called *many*=20
times using a string comparison to denote the type will probably be=20
slow.   Perhaps a template function would be better....

template <enum OutputType>
void output_connectivity(...)

then

elem->output_connectivity<Tecplot> (...)

For the case of GMV which has identical node numbering to some other=20
package (except off-by-one) we can use full specialization:

template <>
void Elem::output_connectivity<GMV> (...)
{
   Elem::output_connectivity<SomeOther> (...)
    ...  increment node numbers by one ...
}


However, this approach requires modifying the element class and adding=20
an enum value whenever a new IO format is added.


Perhaps a better option is to have the classes do it internally:  for=20
example, GMVIO would include a private member,

get_connectivity(const Elem*, std::vector<unsigned int>& conn)

that does a switch on elem->type() and implements the proper=20
connectivity for each element type.  I kinda prefer this approach in the=20
interest of keeping proper code in its place.  It will also encourage=20
people to add new formats if they don't have to change a lot of other=20
classes...


> o Is there a reason for the different calling sequences of the
>   tecplot_connectivity and vtk_connectivity methods methods (return
>   values vs. return arguments)?

The tecplot_connectivity method was written first, and is less efficient=20
since returning a vector can cause an unnecessary copy constructor call.=20
  The VTK method should probably be changed to take a reference instead=20
of a pointer.   However, in light of my preceding comments, perhaps both=20
of these should go in another place.

> To me it seems that the renumbering / splitting could be handled more
> generically. What are your thoughts?

Agreed.  What do you think about what I suggest above?

-Ben