dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Sun, Oct 26, 2008 at 07:49:13PM +0000, Garth N. Wells wrote:
> 
> 
> Anders Logg wrote:
> > On Sun, Oct 26, 2008 at 12:12:06PM +0000, Garth N. Wells wrote:
> >>
> >> Anders Logg wrote:
> >>> On Sun, Oct 26, 2008 at 12:50:12PM +0100, DOLFIN wrote:
> >>>> One or more new changesets pushed to the primary dolfin repository.
> >>>> A short summary of the last three changesets is included below.
> >>>>
> >>>> changeset:   5003:f2effd253de3ae72894bdecb768447b8336a5014
> >>>> tag:         tip
> >>>> user:        "Garth N. Wells <gnw20@xxxxxxxxx>"
> >>>> date:        Sun Oct 26 11:50:01 2008 +0000
> >>>> files:       dolfin/ale/HarmonicSmoothing.cpp dolfin/mf/MatrixFactory.cpp
> >>>> description:
> >>>> DOLFIN compiles again!
> >>> Nice!
> >>>
> >> We need to have a good look at const in the mesh classes. I used quite a 
> >> bit of cont_cast to get things worker because it's always easier to 
> >> start with something that compiles (and we can continue with the 
> >> Function interface if it compiles). A few things:
> > 
> > Good. I agree it's best to get it to compile, then try removing all
> > the const_casts.
> > 
> >> - Do we want to be able to call functions like Mesh::order on const 
> >> meshes? I would suggest yes because it's nice to make objects like Form 
> >>   const because they are 95% const, but it is necessary to call various 
> >> mesh functions. We would need to use mutable where necessary in the mesh 
> >> classes.
> > 
> > I think the init() functions can very well be const (and use
> > const_cast internally). 
> 
> Isn't this when mutable comes in handy? It avoids the const_cast inside 
> init().

Yes, but then we would need to make MeshConnectivity mutable which is
essentially all mesh data except for the coordinates, so then const
has little meaning.

> One may say that for example the edges of a
> > Mesh always exist, even though that may not have been computed. So,
> > one should be able to do
> > 
> >   const Mesh& mesh;
> >   for (EdgeIterator edge(mesh); !edge.end(); ++edge)
> >   {
> > 
> >   }
> > 
> > even if this may potentially lead to a precomputation of all edges at the
> > start of the loop.
> > 
> > However, I don't think it's a good idea to make Mesh::order() const,
> > since it may potentially change the numbering of the mesh. So a user
> > could then assemble over a mesh and as a result the mesh will have
> > changed, even though the assemble() function promises to be const
> > regarding the Mesh (Form).
> > 
> > This is problematic considering we now automatically initialiaze
> > edges, faces etc and make sure they are ordered. What I think we need
> > to do is to check whether or not the mesh is ordered and if not so
> > give an error message saying that the mesh has not been correctly
> > ordered. In those cases, a user will need to do
> > 
> >   mesh.order()
> > 
> > and then call assemble().
> > 
> 
> I'm wouldn't be very happy with this solution because I it breaks the 
> simple interface. It's not intuitive that a user should call mesh.order().
> 
> No ideal, but what if we don't pass 'const Form' to Assemblerm but just 
> 'Form' instead. We have a degree of control by adding only const member 
> functions to Form with exceptions as needed.

I think one should be able to expect that the Form is not changed when
something is assembled from it.

I've added a check in DofMap::init() instead of calling order(). Let's
see if it causes any trouble. I think it's a good thing that we don't
hide the reordering of the mesh. It may be very confusing to try to
debug an application if the mesh may be silently reordered.

We could possibly use a const_cast in DofMap::init() and issue a
warning that the mesh has been renumbered.

> >> - We should add cont version of the various mesh iterators.
> > 
> > Do we need that? My idea with the mesh iterators (now) was that they
> > are always const in the sense that they can be created from a const
> > Mesh or MeshEntity. This means that one cannot use an iterator to
> > modify the mesh. The only place I can think of where this makes sense
> > (except perhaps internally in TopologyComputation) is to change
> > coordinates, and in that case we can require that one uses
> > mesh.geometry().
> > 
> 
> It was in MeshSmoothing that I ran into trouble and added a const iterator.

ok, I'll take a look.

> I would be nice to prioritise what we need to do to get DOLFIN working 
> again so we don't digress too far from this.

I agree, but would like to spend a few more hours on working out the
constness.

-- 
Anders

Attachment: signature.asc
Description: Digital signature