Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update staggered grids implementation in various examples #1954

Merged
merged 11 commits into from
Mar 9, 2020

Conversation

johnomotani
Copy link
Contributor

As requested in #1925 (comment), this PR updates the examples which previously used CtoL and LtoC operators. Most if not all of the examples probably still have issues if staggered grids support is turned on, but at least what is there now uses up to date operators.

johnomotani and others added 10 commits March 7, 2020 18:31
This is a best-effort attempt, and is probably incomplete. Removes the
use of CtoL and LtoC operators.

elm-pb is run by default with the option mesh:StaggerGrids=false.
Running elm-pb with staggered grids should (still) be regarded as highly
experimental and likely to fail.
This is a best-effort update, and may not work correctly. By default the
option mesh:StaggerGrids=false for reconnect-2field, and staggered grid
support should be regarded as experimental. In particular, BOUT++'s
Grad_parP operator does not support staggered grids, and so the
implementation here for a modified Grad_parP of calling the built-in
Grad_parP and then interpolating the result may not be a very good
choice.
Use of CtoL and LtoC differencing was previously switched on by default,
so now mesh:StaggerGrids=true by default. Not tested, but as long as
only closed flux surfaces are simulated, the staggering ought to work.
This is a best-effort update, and may not work correctly. By default the
option mesh:StaggerGrids=false for dalf3, and staggered grid support
should be regarded as experimental.
Staggering should now be enabled by setting mesh:StaggerGrids=true and
using the default operators. Note use of staggered grids is off by
default, and not tested.
This is a best-effort update, and may not work correctly. By default the
option mesh:StaggerGrids=false for gyro-gem, and staggered grid support
should be regarded as experimental.
This is a best-effort update, and may not work correctly. By default the
option mesh:StaggerGrids=false for gravity_reduced, and staggered grid
support should be regarded as experimental.
Since headers were renamed, need to change interpolation.hxx to
interpolation_xz.hxx to provide the interpolate() function.
Just disabelling staggering results in errors.
Now also the current settings work with staggering.
Copy link
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabling staggered grids results in errors, if we are setting staggered locations.

It would be great if all of the examples

  • compile (we have a test for this)
  • run without error (easy to test)
  • produce decent output (much harder to test)

I am perfectly happy to reduce the number of examples, if we can get them in better shape.
This way new users will pick one that works, and are more likely to use BOUT++

examples/6field-simple/elm_6f.cxx Show resolved Hide resolved
examples/elm-pb/elm_pb.cxx Outdated Show resolved Hide resolved
@johnomotani
Copy link
Contributor Author

Thanks @dschwoerer!

@dschwoerer fixed elm-pb to work with staggered grids either enabled or disabled. Most of the other examples need a similar fix, or else remove all the staggering-related bits, namely: 6field-simple, dalf3, gravity_reduced, gyro-gem, jorerk-compare, and reconnect-2field. I don't know if any of these examples work with staggering, and they all have it disabled in their example input files, so I would suggest the simplest thing would be to remove the staggering altogether.

@ZedThree
Copy link
Member

ZedThree commented Mar 9, 2020

I agree that the examples could do with a spring clean and check that they give sensible outputs. We should also upgrade them all to use PhysicsModel.

I'll try and at least check everything runs this week.

@bendudson bendudson merged commit 9e1c651 into next Mar 9, 2020
@bendudson bendudson deleted the examples-staggering-update branch March 9, 2020 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants