On Tue, Jan 14, 2014 at 08:08:50PM +0000, Garth N. Wells wrote: > On 2014-01-14 19:28, Anders Logg wrote: > >On Tue, Jan 14, 2014 at 05:19:55PM +0000, Garth N. Wells wrote: > >>On 2014-01-14 16:24, Anders Logg wrote: > >>>On Mon, Jan 13, 2014 at 07:16:01PM +0000, Garth N. Wells wrote: > >>>>On 2014-01-13 18:42, Anders Logg wrote: > >>>>>On Mon, Jan 13, 2014 at 12:45:11PM +0000, Garth N. Wells wrote: > >>>>>>I've just pushed some changes to master, which for > >>>>>> > >>>>>> from dolfin import * > >>>>>> mesh = UnitCubeMesh(128, 128, 128) > >>>>>> mesh.init(2) > >>>>>> mesh.init(2, 3) > >>>>>g> > >>>>>>give a factor 2 reduction in memory usage and a factor 2 speedup. > >>>>>>Change is at > >>>>>>https://bitbucket.org/fenics-project/dolfin/commits/8265008. > >>>>>> > >>>>>>The improvement is primarily due to d--d connectivity no being > >>>>>>computed. I'll submit a pull request to throw an error when d--d > >>>>>>connectivity is requested. The only remaining place where d--d is > >>>>>>(implicitly) computed is in the code for finding constrained mesh > >>>>>>entities (e.g., for periodic bcs). The code in question is > >>>>>> > >>>>>> for (MeshEntityIterator(facet, dim) e; . . . .) > >>>>>> > >>>>>>when dim is the same as the topological dimension if the facet. As > >>>>>>in other places, it would be consistent (and the original intention > >>>>>>of the programmer) if this just iterated over the facet itself > >>>>>>rather than all facets attached to it via a vertex. > >>>>> > >>>>>I don't see why an error message is needed. Could we not just add the > >>>>>possibility to specify what d -- d means? It might be useful for other > >>>>>algorithms to be able to compute that data. > >>>>> > >>>> > >>>>I think it should be removed because it's (i) ad-hoc, (ii) is not > >>>>used/required in the library and (iii) is a memory monster. > >>>>Moreover, we have dimension-independent algorithms that work when > >>>>d--d connectivity is a connection from an entity to itself (for > >>>>which we don't need computation and memory eating). We shouldn't > >>>>have an unnecessary, memory monster d-0-d data structure being > >>>>created opaquely for no purpose, which is what what > >>>> > >>>> for (MeshEntityIterator e(entity, dim); . . . .){....} > >>>> > >>>>does at present when (dimension of entity) = dim. The excessive > >>>>memory usage is an issue for big problems. > >>>> > >>>>If a user wants d-0-d it can be built explicitly, which makes both > >>>>the purpose and the intention clear. > >>> > >>>How can it be built explicitly without calling mesh.init(d, d)? > >>> > >> > >>Build d--0, then 0--d: > >> > >> for (MeshEntityIterator e0(mesh, d); .....) > >> for (VertexIterator v(*e0); ....) > >> for (MeshEntityIterator e2(*v, d); .....) > > > >Yes, that's even more explicity but since we already have a function > >that computes exactly that, I don't see the urge to remove it. > > > >>Note that d--0--d is no longer used in DOLFIN (except by accident in > >>finding periodic bcs because of inconsistent behind-the-scenes > >>behaviour, and it should be changed because it's expensive and not > >>required for periodic bcs). > > > >Not true - see line 67 of Extrapolation.cpp. > > That code is not covered by any test or demo.
Yes it is, by a unit test under test/unit/adaptivity/ and all the auto-adaptive demos, including the documented demo auto-adaptive-poisson. > >Having easy access to > >neighboring cells (however one chooses to define what it means to be a > >cell neighbor of a cell) can be useful to many algorithms that perform > >local search algorithms or solve local problems. > > > > Sure, but again the user should decide how a connection is defined. Yes, I agree. > >Again, I don't see the urge to remove this function just because it > >consumes a lot of memory. The important point is to avoid using it for > >standard algorithms in DOLFIN, like boundary conditions. > > That's not a good argument. If poor/slow/memory hogging code is left > in the library, it eventually gets used in the library despite any > warnings. It's happened time and time again. I have never understood this urge to remove all functionality that can potentially be misused. We are adults so it should be enough to write a warning in the documentation. > >The proper > >way to fix functions that should not use this particular function (in > >for example periodic bcs) would be to create an issue for its misuse. > > > > The periodic bc code does not mis-use the function. The flaw is that > the mesh library magically and unnecessarily creates a memory > monster behind the scenes. I agree with that. -- Anders _______________________________________________ fenics mailing list [email protected] http://fenicsproject.org/mailman/listinfo/fenics
