CliMA / Oceananigans.jl

🌊 Julia software for fast, friendly, flexible, ocean-flavored fluid dynamics on CPUs and GPUs
https://clima.github.io/OceananigansDocumentation/stable
MIT License
965 stars 190 forks source link

Strategy for implementing and testing models with vertically stretched grids #471

Closed ali-ramadhan closed 3 years ago

ali-ramadhan commented 4 years ago

I've figured out how to do the 3D pressure solve on a stretched grid following Chris' notes so I think we have all the pieces we need to implement vertically stretched grids, we just have to put it all together.

I'm thinking about how to go about fully incorporating a vertically stretched grid, true finite volume operators, and the new FFT+tridiagonal Poisson solvers needed to solve for the pressure on a stretched grid.

Seems like a good idea to split it up into steps with one pull request per step:

  1. Reverse the k index. Currently PR #462.
  2. Revise the RegularCartesianGrid struct so we're happy with it. Currently PR #464.
  3. Add finite volume operators as a separate piece of code. Technically they won't be tested in this PR and could have mistakes. Currently PR #283
  4. Nuke the old operators and start using the same set of finite volume operators for both Oceananigans.Operators and closure_operators.jl. This will test that the finite volume operators reduce down to the operators that currently work, but doesn't test them on a stretched grid.
  5. Implement a VerticallyStretchedCartesianGrid. Might have to iterate bit to figure out what we need, e.g. I think we'll want ΔzC to include the distance between the first cell center and the halo cell center, etc.
  6. Implement CPU and GPU pressure solvers for vertically stretched grids with tests. There will be two: one for horizontally periodic domains and another for channel models. I've figured most of this stuff out in Jupyter notebooks.
  7. Ensure that models with vertically stretched grids pass basic tests: e.g. incompressibility, tracer conservation, etc. This will test the finite volume operators.
  8. Run a model with a VerticallyStretchedCartesianGrid but with uniform grid spacing and make sure it produces the same numbers as a model with RegularCartesianGrid. This is a sanity check.
  9. Run additional tests for vertically stretched grids: e.g. vertical diffusion, internal wave, etc. This will also test boundary conditions with stretched grids.
  10. Rerun the stratified Couette flow verification experiment but with a stretching factor matching Vreugdenhil & Taylor (2018). This will test the AMD closure on stretched grids.

Let me know if anyone has any thoughts. cc @jm-c @rafferrari

johncmarshall54 commented 4 years ago

Good to see stretched grid etc. Can we check with atmospheric clima model and with emerging DG hydrostatic ocean model, that z index increases upwards there too... best to have consistency across models. Finite volume is good and hopefully need not slow model down. If we do move on to other grids, will be useful.

On Sat, Oct 12, 2019, 5:16 PM Ali Ramadhan notifications@github.com wrote:

I've figured out how to do the 3D pressure solve on a stretched grid following Chris' notes so I think we have all the pieces we need to implement vertically stretched grids, we just have to put it all together.

I'm thinking about how to go about fully incorporating a vertically stretched grid, true finite volume operators, and the new FFT+tridiagonal Poisson solvers needed to solve for the pressure on a stretched grid.

Seems like a good idea to split it up into steps with one pull request per step:

  1. Reverse the k index. Currently PR #462 https://github.com/climate-machine/Oceananigans.jl/pull/462.
  2. Revise the RegularCartesianGrid struct so we're happy with it. Currently PR #464 https://github.com/climate-machine/Oceananigans.jl/pull/464.
  3. Add finite volume operators as a separate piece of code. Technically they won't be tested in this PR and could have mistakes. Currently PR #283 https://github.com/climate-machine/Oceananigans.jl/pull/283
  4. Nuke the old operators and start using the same set of finite volume operators for both Oceananigans.Operators and closure_operators.jl. This will test that the finite volume operators reduce down to the operators that currently work, but doesn't test them on a stretched grid.
  5. Implement a VerticallyStretchedCartesianGrid. Might have to iterate bit to figure out what we need, e.g. I think we'll want ΔzC to include the distance between the first cell center and the halo cell center, etc.
  6. Implement CPU and GPU pressure solvers for vertically stretched grids with tests. There will be two: one for horizontally periodic domains and another for channel models. I've figured most of this stuff out in Jupyter notebooks.
  7. Ensure that models with vertically stretched grids pass basic tests: e.g. incompressibility, tracer conservation, etc. This will test the finite volume operators.
  8. Run a model with a VerticallyStretchedCartesianGrid but with uniform grid spacing and make sure it produces the same numbers as a model with RegularCartesianGrid. This is a sanity check.
  9. Run additional tests for vertically stretched grids: e.g. vertical diffusion, internal wave, etc. This will also test boundary conditions with stretched grids.
  10. Rerun the stratified Couette flow verification experiment but with a stretching factor matching Vreugdenhil & Taylor (2018). This will test the AMD closure on stretched grids.

Let me know if anyone has any thoughts. cc @jm-c https://github.com/jm-c @rafferrari https://github.com/rafferrari

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/climate-machine/Oceananigans.jl/issues/471?email_source=notifications&email_token=AKXUEQULGTW4EOYLESVYBPTQOI5CRA5CNFSM4JAEMMD2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HRMSY6Q, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKXUEQTYMTRQSQIOMRLIHPTQOI5CRANCNFSM4JAEMMDQ .

christophernhill commented 4 years ago

@ali-ramadhan this looks good. Be good to chat next week. The general FV piece intersects with thinking about one day being able to do LES with bathymetry and still some performance.

christophernhill commented 4 years ago

@johncmarshall54 is right that we should try and make sure we all know which direction k+1 is wrt to g. That would be good to also have consistent in the "JULES" effort too!

ali-ramadhan commented 4 years ago

Can we check with atmospheric clima model and with emerging DG hydrostatic ocean model, that z index increases upwards there too... best to have consistency across models.

Last time this came up in https://github.com/climate-machine/Oceananigans.jl/issues/90 it seemed that k increases as you move away from the surface of the Earth (although they may have meant the center of the Earth? Otherwise DG ocean and atmosphere would have opposing conventions). Also mentioned was that the unstructured grid means sometimes you just do unstructured stuff and there's no k index I guess.

@blallen Does increasing the vertical k index go up or down for the DG ocean model?

Finite volume is good and hopefully need not slow model down. If we do move on to other grids, will be useful.

Yeah for uniform grids it shouldn't. We have benchmarks we can check against. Things might slow down a bit with z operators, but that's to be expected. If we do shared memory right, it might not be by a huge amount.

Be good to chat next week. The general FV piece intersects with thinking about one day being able to do LES with bathymetry and still some performance.

That would be good, haven't though much about topography. I'll be around.

blallen commented 4 years ago

@blallen Does increasing the vertical k index go up or down for the DG ocean model?

I believe k starts at 1 at the ocean floor and increases going towards the ocean surface. Of course, this is only true for the StackedBrickTopology (e.g. OceanInABox), I'm not sure what the convention is for the CubeSphereTopology.

ali-ramadhan commented 4 years ago

Nice! So hopefully once PR #462 is merged then our conventions will agree (at least for StackedBrickTopology).