boutproject / BOUT-dev

BOUT++: Plasma fluid finite-difference simulation code in curvilinear coordinate systems
http://boutproject.github.io/
GNU Lesser General Public License v3.0
182 stars 95 forks source link

Boundary conditions in staggered grids #411

Open dschwoerer opened 7 years ago

dschwoerer commented 7 years ago

It seems some Boundary conditions change the values within the domain.

Am I wrong in the assumption that the BC should only change the values outside the domain?

I tested with test-minmax and set a few boundary conditions. With none the test succeeds, but it fails with:

bendudson commented 7 years ago

Correct, it should only change values outside the domain

d7919 commented 7 years ago

Is this in rc4 or master (note neumann and neumann_o2 are the same in rc4 I think but different in master)? Is this with 2D and/or 3D fields? What is the location you're using for the staggered grids?

d7919 commented 7 years ago

Looking at this line it looks like the boundary operator is offsetting the index in the field being set when dealing with inner boundaries. I'm not very familiar with the staggering but I'm not sure that this is correct -- I'd have thought we should still set just the indices given by the boundary iterator.

bendudson commented 7 years ago

If a field is shifted to XLOW, then (assuming MXG=2) two points are outside the boundary, and the third point is on the boundary. If it were cell centred then this point would be in the domain, but as it's staggered it's on the boundary.

On 8 December 2016 at 09:56, David Dickinson notifications@github.com wrote:

Looking at this line https://github.com/boutproject/BOUT-dev/blob/v4.0.0-RC/src/mesh/boundary_standard.cxx#L2089 it looks like the boundary operator is offsetting the index in the field being set when dealing with inner boundaries. I'm not very familiar with the staggering but I'm not sure that this is correct -- I'd have thought we should still set just the indices given by the boundary iterator.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/boutproject/BOUT-dev/issues/411#issuecomment-265700000, or mute the thread https://github.com/notifications/unsubscribe-auth/AANYYVTG_K_ov7CIQAyWJ0KBDz3F0XHXks5rF9RHgaJpZM4LHRU1 .

d7919 commented 7 years ago

Ah ok thanks, does this mean that RGN_NOBNDRY should skip this point (and if so does it)?

bendudson commented 7 years ago

Yes, it probably should skip that. At the moment Field3D::region doesn't take account of staggered fields. Should be a straightforward fix to check cell location there.

On 8 December 2016 at 10:16, David Dickinson notifications@github.com wrote:

Ah ok thanks, does this mean that RGN_NOBNDRY should skip this point (and if so does it)?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/boutproject/BOUT-dev/issues/411#issuecomment-265704302, or mute the thread https://github.com/notifications/unsubscribe-auth/AANYYUPb7SNvly4zIhZYkvExUZGF5ODfks5rF9jpgaJpZM4LHRU1 .

d7919 commented 7 years ago

OK, so this is probably the reason that test-minmax is impacted -- min and max use a region iterator to try to avoid the boundaries but if fields are staggered then this will include one of the boundary points.

dschwoerer commented 7 years ago

I think it may be a bit more complicated. The staggered grids have a different number of guards cells:

1:   .   .   x   x   x   .   .
2: .   .   x   x   x   .   .
3: .   .   v   x   x   v   .
4: .   .   v   x   v   .   .
5:   .   .   x   x   .   .   .

1: is the non staggered case. 2: The current interpretation 3: The way it currently is where x is in the domain, . is a boundary cell and v is on the boundary. Note that the number of guard cells is not symmetric in the staggered case. I am not sure right now whether this means there should be one more guard cell, or whether we have a spare one ... In case we want to set the value at the boundary v should be changed, however if we want to set the derivative, v shouldn't be changed, but .. In case of 1 guard cell, there would be no cell outside, and we cannot set the derivative ...

Update: 4: The way it could be, would allow enough boundary cells for staggered grids 5: The non-staggered grid, compatible with option 4

dschwoerer commented 7 years ago

I think in terms of taking derivatives, if the derivative is taken from the staggerd to the non staggered, the boundary cells are sufficient. However, if we want to take the derivative and have that derivative on the staggered, we need more boundary cells.

Further if the C2 scheeme is used, it is not really possible to set the derivative at the boundary, if we calculate from stag->nonstag - not sure whether this is a bug, or the way it should be ...