Oceananigans.jl: Which topologies are actually supported?

Created on 23 Nov 2020  Â·  10Comments  Â·  Source: CliMA/Oceananigans.jl

Not sure if I should open another issue for this, but I was trying to set-up a 2D topology like (Bounded, Flat, Bounded) but couldn't. Apparently, however, (Flat, Bounded, Bounded) works.

Is there a list of available topologies to choose from? The topology page on the docs leads me to believe that they're all implemented but (unless I'm missing something) they're not. Is that list somewhere in the docs? I couldn't find it.

Cheers!

_Originally posted by @tomchor in https://github.com/CliMA/Oceananigans.jl/issues/1178#issuecomment-732235261_

question 💭

Most helpful comment

Sorry about the confusion @tomchor! Opening a new issue might actually be a good idea since it's true we currently only support a subset of the 3^3 = 27 possible topologies (I went ahead and quoted your post in this new issue).

I actually might avoid using the Flat topology right now since until it's fully tested (see e.g. #1023).

You can substitute Bounded for Flat and your simulation should do the same thing except it will take up some extra memory and slow down your model by a tiny bit. The functional difference is that halo regions have non-zero size along Bounded dimensions (and derivatives are computed along Bounded dimensions which will evaluate to zero unless you have some weird boundary conditions).

Yeah currently the four supported topologies are:

  1. (Periodic, Periodic, Periodic)
  2. (Periodic, Periodic, Bounded)
  3. (Periodic, Bounded, Bounded)
  4. (Bounded, Bounded, Bounded) (unfortunately only works on the CPU, see #1007).

We could support all possible topology combinations. The only bottleneck is implementing pressure solver that work for all topologies (see #586). This is on my current list of things to do but is not super trivial (see https://github.com/JuliaGPU/CUDA.jl/issues/119 and possibly related #1170) so it's taking some time... 😅

Hopefully I can refactor the pressure solver(s) to support all topologies soon but either way it might be good to be explicit about which topologies are actually supported.

All 10 comments

Sorry about the confusion @tomchor! Opening a new issue might actually be a good idea since it's true we currently only support a subset of the 3^3 = 27 possible topologies (I went ahead and quoted your post in this new issue).

I actually might avoid using the Flat topology right now since until it's fully tested (see e.g. #1023).

You can substitute Bounded for Flat and your simulation should do the same thing except it will take up some extra memory and slow down your model by a tiny bit. The functional difference is that halo regions have non-zero size along Bounded dimensions (and derivatives are computed along Bounded dimensions which will evaluate to zero unless you have some weird boundary conditions).

Yeah currently the four supported topologies are:

  1. (Periodic, Periodic, Periodic)
  2. (Periodic, Periodic, Bounded)
  3. (Periodic, Bounded, Bounded)
  4. (Bounded, Bounded, Bounded) (unfortunately only works on the CPU, see #1007).

We could support all possible topology combinations. The only bottleneck is implementing pressure solver that work for all topologies (see #586). This is on my current list of things to do but is not super trivial (see https://github.com/JuliaGPU/CUDA.jl/issues/119 and possibly related #1170) so it's taking some time... 😅

Hopefully I can refactor the pressure solver(s) to support all topologies soon but either way it might be good to be explicit about which topologies are actually supported.

Thanks!
I'm a bit confused though. I see some topologies in the examples that run successfully but that aren't in your list. For example the Langmuir example has this grid:

RegularCartesianGrid{Float64, Periodic, Periodic, Bounded}
                   domain: x ∈ [0.0, 128.0], y ∈ [0.0, 128.0], z ∈ [-96.0, 0.0]
                 topology: (Periodic, Periodic, Bounded)
  resolution (Nx, Ny, Nz): (32, 32, 48)
   halo size (Hx, Hy, Hz): (1, 1, 1)
grid spacing (Δx, Δy, Δz): (4.0, 4.0, 2.0)

Is it possible that you're listing the topologies in the z, y, x format instead of x, y, z?

Ah sorry you're right, good catch! I updated the original list with the actually supported topologies.

Although I guess a side note worth mentioning is that I think you can create a grid with any topology and it won't error until it tries to construct a pressure solver. Might be clearer and more user friendly to print a useful error/warning when constructing the grid.

Yes, in my tests I only get an error when getting the pressure solver going. And I agree that an error when actually constructing the grid would be better! Although I think updating the docs is the priority. Would you like me to update it using your list in the previous comment?

If you have the time to add the list of supported topologies to the documentation that would be very much appreciated!

Just included it here: https://github.com/CliMA/Oceananigans.jl/pull/1193

This is my first time contributing so hopefully I followed the right protocol!

Looks great, thank you for the PR!

We've recently begun to follow the ColPrac guide (also PR #1044) which has some pretty nice guidelines on community package development.

Should this be closed because of #1193 ?

Ah yes!

PS: If you add something like "Resolves #1192" to your PR then merging the PR will automatically the associated issue(s).

Awesome! Thanks for the heads-up. I was wondering how to link those to their issues.

Cheers!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ali-ramadhan picture ali-ramadhan  Â·  5Comments

ali-ramadhan picture ali-ramadhan  Â·  6Comments

glwagner picture glwagner  Â·  6Comments

ali-ramadhan picture ali-ramadhan  Â·  5Comments

ali-ramadhan picture ali-ramadhan  Â·  8Comments