dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Sun, Oct 26, 2008 at 08:23:37PM +0000, Garth N. Wells wrote:
> 
> 
> Anders Logg wrote:
> > 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.
> >
> 
> ok
> 
> >> 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.
> >
> 
> Can you remind why we need to re-order? Is it because we don't require a 
> connectivity order in the input mesh?
> 
> It is rare that it's needed, I don't mind an error message if 
> mesh.order() is required for only special meshes (e.g., possible those 
> not created by dolfin-convert)
> 
> >>>> - 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.
> > 
> 
> We'll give you a few hours ;). A few days/weeks/months/years would have 
> been a problem.
> 
> Garth

Thanks... ;-)

-- 
Anders

Attachment: signature.asc
Description: Digital signature