[Dune] Returning Geometries As Objects
Steffen Müthing
steffen.muething at ipvs.uni-stuttgart.de
Wed Feb 8 20:56:45 CET 2012
Hi Martin,
Am 08.02.2012 um 20:00 schrieb Martin Nolte:
> Hi Steffen,
>
> what you say is true: Such code might break in a very subtle way. Unfortunately, the compiler seems not to be able to warn when caching such a reference to a temporary. However, I think this problem will only occur in discretization frameworks, and, hence is under our direct control. In dune-fem, geometry references were only stored in very few places.
>
I also think that the problem is pretty isolated, I just hadn't really thought about it before...
Nevertheless, we should probably document the issue somewhere where our users are not
likely to miss it.
> The issue concerning meta grids is not really a problem. Only very few people have ever created their own grid and they shouldn't have any problem adapting their code. Moreover, I'm very willing to help in case of problems. I would also like to point out that, having already changed such a meta grid (IdGrid), the code looks much nicer and clearer, now. And for the 2.5 guys who actually care: It is also faster.
Let me be clear: I would absolutely like this change to occur, mostly *because* it will clean up
MultiDomainGrid! :-)
I just wanted to make sure all metagrid maintainers are aware of the fact that they will have to look
into their grids to avoid subtle problems. And about the speed: I'm also pretty sure that MultiDomainGrid
will be faster with the new geometries.
But I thought I should raise the issue - it will require more attention when we start working on
converting the Entities, which more people might be storing in class instances.
Best,
Steffen
>
> Best,
>
> Martin
>
>
> On 02/08/2012 07:05 PM, Steffen Müthing wrote:
>> Hi all,
>>
>> Am 25.01.2012 um 09:49 schrieb Martin Nolte:
>>
>>> Dear all,
>>>
>>> as announced in December, there is now an implementation returning the geometry as an object and all grid implementations in dune-grid have been adapted accordingly. Therefore, I think time has come for discussion of the outcome.
>>>
>>
>>>
>>> As already mentioned in the announcement of the transition, there is also a subtle impact on user code. Geometry references may no longer be stored outside the scope calling the geometry method. For example:
>>>
>>> const Geometry *pgeo = 0;
>>> {
>>> const Geometry&geo = entity.geometry();
>>> pgeo =&geo;
>>> }
>>>
>>> worked with the current interface but will break after the transition. Notice that the compiler will now warn in such a situation and the resulting code will probably segfault. The good news is that the segfault is extremely likely (since, e.g., pgeo now points onto the execution stack). However, such code will be rare and probably only found in the higher discretization modules, such as dune-fem, dune-fufem, or dune-pdelab.
>>
>>
>> I just realized that this problem can also come up when the reference is a
>> class member, as might happen in a caching structure of a discretization
>> module (like the first argument passed to the local operator in PDELab),
>> and any code with a cache object like this:
>>
>> template<typename GV>
>> class Cache
>> {
>>
>> Cache(const Entity& e)
>> : geometry(e.geometry())
>> {}
>>
>> const Geometry& geometry;
>>
>> };
>>
>> will probably break, as the compiler will destroy the temporary returned by
>> the call to Entity::geometry() as soon as the constructor exits (cf. section
>> 12.2.5 of the standard). As Martin pointed out, the referenced address is
>> on the stack and will probably be overwritten pretty quickly, but I'm not sure
>> whether that will always cause a direct segfault or instead taunt us with subtle
>> and hard-to-track bugs. And unfortunately, we don't get a compiler warning
>> in this case (at least not with plain -Wall).
>>
>> In PDELab we do not run into this problem right now because we only cache
>> the entity and not the geometry, but I don't know about the other assembler
>> frameworks out there.
>>
>> The second area where this issue might show up is in metagrids that simply
>> reuse the underlying geometry - in that case, the Geomety wrapper needs to
>> somehow store the geometry, and as it can only do that by reference or using
>> a pointer, all metagrid maintainers should probably check whether their grid
>> works using Martin's branch (I admit, I haven't done any checking on
>> MultiDomainGrid yet, but I am pretty sure that my grid will have to be modified
>> to work with this change - will do some testing tomorrow).
>>
>> Even if we decide that the issue is not a real problem for the geometries, I'm afraid
>> that it might cause a lot of major breakage in user code once we start the conversion
>> to value-typed Entities (I do hope most everyone caches those, or all of our courses
>> have been in vain ;-) ).
>>
>> As I said, I don't know how dangerous this is in reality (especially for the geometries),
>> but I thought I'd better bring it up...
>>
>>
>> Steffen
>>
>> PS: I attached a very simple example illustrating the issue.
>>
>>
>>
>>
>>
>>
>>
>> Steffen Müthing
>> Universität Stuttgart
>> Institut für Parallele und Verteilte Systeme
>> Universitätsstr. 38
>> 70569 Stuttgart
>> Tel: +49 711 685 88429
>> Fax: +49 711 685 88340
>> Email: steffen.muething at ipvs.uni-stuttgart.de
>>
>>
>>
>>
>>
>>
>> _______________________________________________
>> Dune mailing list
>> Dune at dune-project.org
>> http://lists.dune-project.org/mailman/listinfo/dune
>
> --
> Dr. Martin Nolte <nolte at mathematik.uni-freiburg.de>
>
> Universität Freiburg phone: +49-761-203-5630
> Abteilung für angewandte Mathematik fax: +49-761-203-5632
> Hermann-Herder-Straße 10
> 79104 Freiburg, Germany
>
> _______________________________________________
> Dune mailing list
> Dune at dune-project.org
> http://lists.dune-project.org/mailman/listinfo/dune
Steffen Müthing
Universität Stuttgart
Institut für Parallele und Verteilte Systeme
Universitätsstr. 38
70569 Stuttgart
Tel: +49 711 685 88429
Fax: +49 711 685 88340
Email: steffen.muething at ipvs.uni-stuttgart.de
More information about the Dune
mailing list