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
184 stars 95 forks source link

Field2D y-derivatives don't use yup/ydown in next #252

Closed d7919 closed 6 years ago

d7919 commented 8 years ago

In the next branch the field2d::setYStencil routine still uses the "old" approach -- this should be updated to be consistent with field3d

dschwoerer commented 8 years ago

I am in favour of undoing this change in field3d, as it is broken, see. #253

d7919 commented 8 years ago

Note, this means that when a Field2D gets implicity cast/promoted to a 3D we need to do further communication of the new 3D field in order to by able to take y-derivatives (rather than copying the yup/ydown fields, which don't exist in a 2D).

dschwoerer commented 8 years ago

I don't mind having field2d/3d have the yup/ydown. I just think the current implementation is broken, and the most easy way I can see is having FCI as its own mesh. This requires to remove the (anyway slow) set?Stencil functions ...

bendudson commented 8 years ago

Agreed that set stencil functions are not need anymore, and should be replaced by data iterators.

The yup, ydown fields are not just for FCI, but also when shifted metrics are used, which is pretty much all Tokamak simulations. The alternative to storing yup and ydown is to do zshifting for every x derivative operator, as done in master. If no shifting is needed, then yup and ydown just point to the same field, and there should be no overhead. It's possible there's a better way to incorporate standard meshes, shifted metrics and FCI with minimal differences between them though. Now is the time to choose the scheme before next becomes master...

-------- Original Message -------- From:dschwoerer notifications@github.com Sent:Wed, 02 Nov 2016 18:17:04 +0000 To:boutproject/BOUT-dev BOUT-dev@noreply.github.com Subject:Re: [boutproject/BOUT-dev] Field2D y-derivatives don't use yup/ydown in next (#252)

I don't mind having field2d/3d have the yup/ydown. I just think the current implementation is broken, and the most easy way I can see is having FCI as its own mesh. This requires to remove the (anyway slow) set?Stencil functions ...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/boutproject/BOUT-dev","title":"boutproject/BOUT-dev","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/boutproject/BOUT-dev"}},"updates":{"snippets":[{"icon":"PERSON","message":"@dschwoerer in #252: I don't mind having field2d/3d have the yup/ydown.\r\nI just think the current implementation is broken, and the most easy way I can see is having FCI as its own mesh.\r\nThis requires to remove the (anyway slow) set?Stencil functions ..."}],"action":{"name":"View Issue","url":"https://github.com/boutproject/BOUT-dev/issues/252#issuecomment-257953322"}}}

d7919 commented 6 years ago

Is this going to make v4.2 / will it be pushed back or is this not an issue?

ZedThree commented 6 years ago

Fixed in 1948f60734379dfb92b10a00f9949b54a06ccdcc