Adds tests for Poisson solvers with Flat topologies

[glwagner]

No description provided.

[francispoulin]

Thanks for doing this and hope all the tests pass on cpus and gpus!

[glwagner]

GPU tests fail as expected!

[2021/04/08 16:37:27.123] INFO      Testing (Flat, Bounded, Bounded) topology on square grids [GPU]...
--
  | Divergence-free solution [GPU]: Test Failed at /net/ocean/home/data44/data5/glwagner/.buildkite-agent/builds/sverdrup-8/clima/oceananigans/test/test_poisson_solvers.jl:233
  | Expression: divergence_free_poisson_solution(arch, grid)
  | Stacktrace:
  | [1] top-level scope at /net/ocean/home/data44/data5/glwagner/.buildkite-agent/builds/sverdrup-8/clima/oceananigans/test/test_poisson_solvers.jl:233
  | [2] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1119
  | [3] top-level scope at /net/ocean/home/data44/data5/glwagner/.buildkite-agent/builds/sverdrup-8/clima/oceananigans/test/test_poisson_solvers.jl:213
  | [4] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1119
  | [5] top-level scope at /net/ocean/home/data44/data5/glwagner/.buildkite-agent/builds/sverdrup-8/clima/oceananigans/test/test_poisson_solvers.jl:184

Now to try a fix...

[francispoulin]

I was looking at the errors on cpu-solver_tests and found the message below, followed by a bunch of other errors on the lines below. Does someone know why this is failing? I can take a look at it but thought I'd check to see whether this is understood or not.

Vertically stretched Poisson solver [FACR, CPU, (Flat, Bounded, Bounded)]: Error During Test at /storage7/buildkite-agent/builds/tartarus-mit-edu-7/clima/oceananigans/test/test_poisson_solvers.jl:272
  | Test threw exception
  | Expression: vertically_stretched_poisson_solver_correct_answer(Float64, arch, topo, 8, 8, 1:8)
  | ArgumentError: length(size) must be 2.
  | Stacktrace:
  | [1] validate_tupled_argument(::Tuple{Int64,Int64,Int64}, ::Type{T} where T, ::String, ::Int64; greater_than::Int64) at /storage7/buildkite-agent/builds/tartarus-mit-edu-7/clima/oceananigans/src/Grids/input_validation.jl:24
  | [2] validate_tupled_argument(::Tuple{Int64,Int64,Int64}, ::Type{T} where T, ::String, ::Int64) at /storage7/buildkite-agent/builds/tartarus-mit-edu-7/clima/oceananigans/src/Grids/input_validation.jl:24
  | [3] validate_size(::Type{T} where T, ::Type{T} where T, ::Type{T} where T, ::Tuple{Int64,Int64,Int64}) at /storage7/buildkite-agent/builds/tartarus-mit-edu-7/clima/oceananigans/src/Grids/input_validation.jl:48
  | [4] VerticallyStretchedRectilinearGrid(::Type{T} where T; architecture::CPU, size::Tuple{Int64,Int64,Int64}, x::Tuple{Int64,Int64}, y::Tuple{Int64,Int64}, zF::UnitRange{Int64}, halo::Tuple{Int64,Int64,Int64}, topology::Tuple{DataType,DataType,DataType}) at /storage7/buildkite-agent/builds/tartarus-mit-edu-7/clima/oceananigans/src/Grids/vertically_stretched_rectilinear_grid.jl:50
  | [5] vertically_stretched_poisson_solver_correct_answer(::Type{T} where T, ::CPU, ::Tuple{DataType,DataType,DataType}, ::Int64, ::Int64, ::UnitRange{Int64}) at /storage7/buildkite-agent/builds/tartarus-mit-edu-7/clima/oceananigans/test/test_poisson_solvers.jl:140
  | [6] top-level scope at /storage7/buildkite-agent/builds/tartarus-mit-edu-7/clima/oceananigans/test/test_poisson_solvers.jl:272
  | [7] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1119
  | [8] top-level scope at /storage7/buildkite-agent/builds/tartarus-mit-edu-7/clima/oceananigans/test/test_poisson_solvers.jl:270
  | [9] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1119
  | [10] top-level scope at /storage7/buildkite-agent/builds/tartarus-mit-edu-7/clima/oceananigans/test/test_poisson_solvers.jl:175

[glwagner]

It's an error from the constructor for VerticallyStretchedRectilinearGrid. I would guess that the test uses incorrect syntax in constructing a vertically stretched grid?

[francispoulin]

Thanks @glwagner , I will take a look now and see if it's an easy fix or not. Hopefully it is

[francispoulin]

I think that the problem is that we are passing an integer and not an array. I think the fix should be something like this, as then we are passing something of the right type. Unfortunately, when I try trying the solver test group locally, a bunch of other stuff fails. I'm a little hesitant to push the changes as I fear it might make things worst.

                @test vertically_stretched_poisson_solver_correct_answer(Float64, arch, topo, 8,  8, collect(0:zF))
                @test vertically_stretched_poisson_solver_correct_answer(Float64, arch, topo, 16, 8, collect(0:zF))
                @test vertically_stretched_poisson_solver_correct_answer(Float64, arch, topo, 8, 16, collect(0:zF))
                @test vertically_stretched_poisson_solver_correct_answer(Float64, arch, topo, 8, 11, collect(0:zF))
                @test vertically_stretched_poisson_solver_correct_answer(Float64, arch, topo, 5,  8, collect(0:zF))
                @test vertically_stretched_poisson_solver_correct_answer(Float64, arch, topo, 7, 13, collect(0:zF))

[glwagner]

I think that the problem is that we are passing an integer and not an array.

I disagree and let me clarify my statement that "the test uses incorrect syntax". We are testing topologies that are both two- and three-dimensional, but using syntax to construct VerticallyStretchedRectilinearGrid that is only valid for three-dimensional topologies. To see this, take a look at the vs_topos we test:

Oceananigans.jl/test/test_poisson_solvers.jl

Lines 257 to 266 in 365d00d

	vs_topos = [
	(Periodic, Periodic, Bounded),
	(Periodic, Bounded, Bounded),
	(Bounded, Periodic, Bounded),
	(Bounded, Bounded, Bounded),
	(Flat, Bounded, Bounded),
	(Flat, Periodic, Bounded),
	(Bounded, Flat, Bounded),
	(Periodic, Flat, Bounded)
	]

this list includes two-dimensional topologies like (Flat, Bounded, Bounded) (two-dimensional in y-z).

However, when we construct VerticallyStretchedRectilinearGrid we write

Oceananigans.jl/test/test_poisson_solvers.jl

Line 140 in 365d00d

vs_grid = VerticallyStretchedRectilinearGrid(FT, architecture=arch, topology=topo, size=(Nx, Ny, Nz), x=(0, 1), y=(0, 1), zF=zF)

which only works if there is no Flat dimension.

For example, if the topology is (Flat, Bounded, Bounded), then our syntax should be

vs_grid = VerticallyStretchedRectilinearGrid(FT, architecture=arch, topology=topo, size=(Ny, Nz), y=(0, 1), zF=zF)

I'll fix this up!

[francispoulin]

Thanks @glwagner for clarifying and I see the point. Thanks also in advance for fixing it up.

I encountered the same problem when I wanted to test Flat and in the end separted out the different dimensinal cases. Not ideal but it worked for my simple purposes. Will be nice to have something more elegant in the tests.

[glwagner]

FYI @francispoulin I fixed issues with syntax and the constructor, and found that now the Poisson solver tests legitimately fail for VerticallyStretchedRectilinearGrid. So the FourierTridiagonalPoissonSolver currently does not work with Flat dimensions.

I think this is a low-priority issue so it may make sense to write a warning / error in the constructor for FourierTridiagonalPoissonSolver constructor and then remove the tests for it. However, I wanted to give you the chance to look into what changes might need to be made to get the FFT + tridiagonal solve working if x or y are Flat if you wanted to.

[francispoulin]

Thanks for putting this together and I agree it's better.

I will look at the tridiagonal solver this afternoon and see if I can figure it out. However, I think that adding a warning and merging the PR is probably a good idea. A fix for the solver can come under another PR