#276 adapt return value wrong

Metadata

Property	Value
Reported by	Christian Engwer (christi@conan.iwr.uni-heidelberg.de)
Reported at	Mar 25, 2007 21:00
Type	Bug Report
Version	Git (pre2.4) [autotools]
Operating System	Unspecified / All
Last edited by	Christian Engwer (christi@conan.iwr.uni-heidelberg.de)
Last edited at	Oct 7, 2012 13:16

Description

UGGrid<dim>::adapt() always returns true. The Dune documentation states that the return value should be true if the grid got changed, otherwise false.

Not quite. UGGrid::adapt() returns true iff at least one element has been marked for refinement before calling adapt().

Rationale: I didn't see a reasonably simple way to obtain the information whether the grid truely changed from UG. Thus I deliberately made the return value err on the conservative side. The return value is only wrong if you marked elements for refinement but nothing happened. This is a very rare case. The consequences of such a wrong return value are that you assume your grid changed even though it didn't. This may cost you some runtime, but it will not lead to wrong results. Therefore I find the current behaviour acceptable.

When talking about UG one often hears <<Ah! XXXXXXX once wrote something, have a look at YYYYYYY!>>. I think the same i true in this case :-). According to Stefan Lang the macro MGSTATUS(MULTIGRID *mg) should do the trick. The macro should return 1 if nothing changed.

adapt() seems to return the correct value in case of green refinement. In case of hanging nodes refinement the return value is wrong.

What is the status?

Again, what is the status? Seems that nobody cares about this bug.

The status is unchanged. Would it be acceptable to go back to the old behavior (i.e. return true if at least one element has beem marked) for non-conforming refinement?

Sounds good. If one element has been marked it's very well possible that also one element has been refined.

I asked for a fix, because the wrong beaviour broke some of my code. I added some heuristics to work around this bug, but they donÄt hold in general.

What about the MGSTATUS macro?

The macro was used before revision 5188, when Robert commented it out. I don't really know what he intended to do, the patch consists mainly of whitespace changes.

@Oli: As you can see with svn diff -r5187:5188 I added a flag someElementHasBeenMarkedForCoarsening_ and corrected the setting of someElementHasBeenMarkedForRefinement_. I did this because in the adaptation method we use the implementation via multigrid_->status did not give correct results.

Well, does it give the correct results now? Why didn't you close the bug, then?

Well, the status says "reopened". I don't see who did this and I cannot remember but I guess I closed it and it has been reopened for some reason.

If you care to look at the history of this task you will see that Christian reopened it on 2007-05-22. That is roughly two years before your patch 5188.

Well, then indeed I didn't close it. I suggest to close it now if it works for everybody using this code. For me it worked when I implemented it. I suggest to add a test for this return to test-ug.cc. A general adaptation test is not contained in the grid test.

I just checked the docs again. It now states, that the adapt method returns true, if the grid was refined. If I only coarsen my grid, hw can I check if the grid changed at all? Or is te documentation wrong?

Usually you don't need the information whether the grid has changed, but rather whether new cells have been added to the grid. This is due to the adaptation algorithm:

1. call preAdapt (returns true, if some element might be coarsened)
2. if elements might be coarsened, restrict the data as necessary
3. call adapt (returns whether element have been refined)
4. if element have been refined, prolong the data as necessary
5. call postAdapt

As you can see, the return value of preAdapt and adapt tells me whether I have to restrict or prolong any data. Since I might save a grid walkthrough due to this information, this is exactly what I need. Would adapt return true in case of pure coarsening, I might waste one grid walkthrough.

I know, I'm merely asking how to obtain the information, whether my grid has changed without iterating over the grid.

Although it is not the classic adaptation scenario, it is still a valid application:

• I want to coarsen my grid due to some local criterion, but not coarser than, e.g. level L.
• I loop through my grid and call mark, where ever a cell can be coarsened.
• If all child-cells are marked, the cell is actually coarsened.
• Even if the grid doesn't change any more, I'll usually still find cell, which might (in them self) be coarsened.

I see three options: a) mark this as won't fix b) change the behaviour of preAdapt() and make the return value exact (might be very expensive) c) make the information somwhere else available (e.g. has-changed return value of postAdapt)

Maybe we could use Robert's suggestion of a sequence number (name to be found). The idea is simple: Each grid holds a counter (starting at, e.g., 0) that is incremented whenever the grid changes.

This number can, e.g., be used for tagging data files, so that on reloading them, the user can check that they actually match the grid. It can also be used to determine whether the grid has changed during adaptation.

If I understand Robert's idea correctly, different sequence number shall mean different grids, while the opposite need not in general be true (though this is likely to lead to performance losses).

@Christian: The return value of the adapt method should returns true if elements were refined. This way one knows that data has to be prolonged. For coarsening this is returned be preAdapt because this has to be done before adapt is called. To find out whether you grid has changed due to coarsening (in case refined = false) is to check the grid size before and after adapt. Most probably you will have size^n+1 < size^n. Would this solve your problem?

As suggested by Robert the coarsening information is currently by the size (read: the sum of all level sizes). So the discussion is only about if it is natural to have 'was refined' as return value and if summing the level sizes is comfortable. For me it seems that having separate methods wasRefined(), wasCoarsened() is more user friendly and intuitive.

On the other hand this is would be a contradiction to the "slim interface" paradigm of DUNE.

We could just put a note on how to get 'wasCoarsened' in the adapt() docs. Then one could ask why we don't implement it generically but leave it to the user. Sometimes 'slim' and 'nice' are contradictory ;-(

@Robert: Checking the size is exactly what I am doing right now. In general this doesn't mean the grid didn't change. In my specific case it is sufficient, but unsatisfactory. @Carsten: The problem is that there is no way to obtain the information, that the grid was indeed coarsened. The return value of preAdapt() is only a "mightBeCoarsened"

But perhaps we should move this to a different issue. THe problem with the return value of adapt() seems to be solved.

I agree with Christian, that to obtain the that the grid was indeed coarsened is not so easy at all. In ALUGrid and AlbertaGrid this would be possible by using the callback adaptation. For other grids using the standard adaptation mechanism this information is at all not easy to obtain and we should not introduce this in the interface until the whole adaptation interface is more developed then right now.

I would also close this task with "fixed" tag.