← Back to team overview

dolfin team mailing list archive

Re: [HG DOLFIN] DOLFIN compiles again!

 



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



------------------------------------------------------------------------

_______________________________________________
DOLFIN-dev mailing list
DOLFIN-dev@xxxxxxxxxx
http://www.fenics.org/mailman/listinfo/dolfin-dev


Follow ups

References