Thread Previous • Date Previous • Date Next • Thread Next |
Anders Logg wrote:
On Sun, Oct 26, 2008 at 07:49:13PM +0000, Garth N. Wells wrote:Anders Logg wrote:Isn't this when mutable comes in handy? It avoids the const_cast inside init().On Sun, Oct 26, 2008 at 12:12:06PM +0000, Garth N. Wells wrote:Anders Logg wrote: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: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!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 useconst_cast internally).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 aI'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().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().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
Thread Previous • Date Previous • Date Next • Thread Next |