Test that output is saved in cubed sphere simulations

[glwagner]

cc @siddharthabishnu this PR tests that output is saved correctly when running simulations with the cubed sphere. Previously, we tried to build a FieldTimeSeries but did not test that it had any data inside.

[glwagner]

Note, I get an error when I try to run the tests locally:

Testing conformal cubed sphere grid from file: Error During Test at /Users/gregorywagner/Projects/Oceananigans.jl/test/test_multi_region_cubed_sphere.jl:169
  Got exception outside of a @test
  EOFError: read end of file
  Stacktrace:
    [1] jldopen(fname::String, wr::Bool, create::Bool, truncate::Bool, iotype::Type{JLD2.MmapIO}; fallback::Type{IOStream}, compress::Bool, mmaparrays::Bool, typemap::Function, parallel_read::Bool, plain::Bool)
      @ JLD2 ~/.julia/packages/JLD2/SgtOb/src/JLD2.jl:231
    [2] jldopen
      @ ~/.julia/packages/JLD2/SgtOb/src/JLD2.jl:165 [inlined]
    [3] #jldopen#23
      @ ~/.julia/packages/JLD2/SgtOb/src/JLD2.jl:286 [inlined]
    [4] jldopen
      @ ~/.julia/packages/JLD2/SgtOb/src/JLD2.jl:279 [inlined]
    [5] (OrthogonalSphericalShellGrid{FT, TX, TY, TZ, CZ, <:Oceananigans.OrthogonalSphericalShellGrids.CubedSphereConformalMapping, CC, FC, CF, FF, Arch} where {FT, TX, TY, TZ, CZ, CC, FC, CF, FF, Arch})(filepath::String, architecture::CPU, FT::Type; panel::Int64, Nz::Int64, z::Tuple{Int64, Int64}, topology::Tuple{DataType, DataType, DataType}, radius::Float64, halo::Tuple{Int64, Int64, Int64}, rotation::Nothing)
      @ Oceananigans.OrthogonalSphericalShellGrids ~/Projects/Oceananigans.jl/src/OrthogonalSphericalShellGrids/conformal_cubed_sphere_panel.jl:67
    [6] OrthogonalSphericalShellGrid (repeats 2 times)
      @ ~/Projects/Oceananigans.jl/src/OrthogonalSphericalShellGrids/conformal_cubed_sphere_panel.jl:55 [inlined]
    [7] macro expansion
      @ ~/Projects/Oceananigans.jl/test/test_multi_region_cubed_sphere.jl:176 [inlined]
    [8] macro expansion
      @ ~/.julia/juliaup/julia-1.11.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/Test/src/Test.jl:1704 [inlined]
    [9] top-level scope
      @ ~/Projects/Oceananigans.jl/test/test_multi_region_cubed_sphere.jl:170
   [10] include(fname::String)
      @ Main ./sysimg.jl:38
   [11] top-level scope
      @ REPL[1]:1
   [12] eval
      @ ./boot.jl:430 [inlined]
   [13] eval_user_input(ast::Any, backend::REPL.REPLBackend, mod::Module)
      @ REPL ~/.julia/juliaup/julia-1.11.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/REPL/src/REPL.jl:245
   [14] repl_backend_loop(backend::REPL.REPLBackend, get_module::Function)
      @ REPL ~/.julia/juliaup/julia-1.11.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/REPL/src/REPL.jl:342
   [15] start_repl_backend(backend::REPL.REPLBackend, consumer::Any; get_module::Function)
      @ REPL ~/.julia/juliaup/julia-1.11.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/REPL/src/REPL.jl:327
   [16] run_repl(repl::REPL.AbstractREPL, consumer::Any; backend_on_current_task::Bool, backend::Any)
      @ REPL ~/.julia/juliaup/julia-1.11.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/REPL/src/REPL.jl:483
   [17] run_repl(repl::REPL.AbstractREPL, consumer::Any)
      @ REPL ~/.julia/juliaup/julia-1.11.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/REPL/src/REPL.jl:469

@siddharthabishnu why do our tests rely critically on loading cubed sphere from file? I thought we can generate the cubed sphere at any resolution? I would even suggest that we should not support loading grid data from a file, I don't really see the point of trying to support that in the long run.

[siddharthabishnu]

@glwagner You should be able to generate a cubed sphere grid at any resolution (that’s what I’ve been doing all along), rather than only loading it from a file. Loading from a file happens only in the test script for verification purpose, and is just an additional functionality. If you’re seeing an error, it’s likely due to the latest merges into the main branch. I’m a bit busy over the next week, but I’ll look into it as soon as I can.

[glwagner]

@glwagner You should be able to generate a cubed sphere grid at any resolution (that’s what I’ve been doing all along), rather than only loading it from a file. Loading from a file happens only in the test script for verification purpose, and is just an additional functionality. If you’re seeing an error, it’s likely due to the latest merges into the main branch. I’ll need to look into it. I’m a bit busy over the next week, but I’ll look into it as soon as I can.

Tests pass on main, just not locally. But given this is a complication, is there any need to continue supporting loading from file?

[siddharthabishnu]

@glwagner You should be able to generate a cubed sphere grid at any resolution (that’s what I’ve been doing all along), rather than only loading it from a file. Loading from a file happens only in the test script for verification purpose, and is just an additional functionality. If you’re seeing an error, it’s likely due to the latest merges into the main branch. I’ll need to look into it. I’m a bit busy over the next week, but I’ll look into it as soon as I can.

Tests pass on main, just not locally. But given this is a complication, is there any need to continue supporting loading from file?

It’s just this test function where the cubed sphere grid is loaded from a file:

@testset "Testing conformal cubed sphere grid from file" begin
    Nz = 1
    z = (-1, 0)

    cs32_filepath = datadep"cubed_sphere_32_grid/cubed_sphere_32_grid_with_4_halos.jld2"

    for panel in 1:6
        grid = ConformalCubedSpherePanelGrid(cs32_filepath; panel, Nz, z)
        @test grid isa OrthogonalSphericalShellGrid
    end

    for arch in archs
        @info "  Testing conformal cubed sphere grid from file [$(typeof(arch))]..."

        # read cs32 grid from file
        grid_cs32 = ConformalCubedSphereGrid(cs32_filepath, arch; Nz, z)

        radius = first(grid_cs32).radius
        Nx, Ny, Nz = size(grid_cs32)
        Hx, Hy, Hz = halo_size(grid_cs32)

        Nx !== Ny && error("Nx must be same as Ny")
        N = Nx
        Hx !== Hy && error("Hx must be same as Hy")
        H = Hy

        # construct a ConformalCubedSphereGrid similar to cs32
        grid = ConformalCubedSphereGrid(arch; z, panel_size=(Nx, Ny, Nz), radius,
                                        horizontal_direction_halo = Hx, z_halo = Hz)

        for panel in 1:6
            @allowscalar begin
                # Test only on cca and ffa; fca and cfa are all zeros on grid_cs32!
                # Only test interior points since halo regions are not filled for grid_cs32!

                @test compare_grid_vars(getregion(grid, panel).φᶜᶜᵃ, getregion(grid_cs32, panel).φᶜᶜᵃ, N, H)
                @test compare_grid_vars(getregion(grid, panel).λᶜᶜᵃ, getregion(grid_cs32, panel).λᶜᶜᵃ, N, H)

                # before we test, make sure we don't consider +180 and -180 longitudes as being "different"
                getregion(grid, panel).λᶠᶠᵃ[getregion(grid, panel).λᶠᶠᵃ .≈ -180] .= 180

                # and if poles are included, they have the same longitude.
                same_longitude_at_poles!(grid, grid_cs32)

                @test compare_grid_vars(getregion(grid, panel).φᶠᶠᵃ, getregion(grid_cs32, panel).φᶠᶠᵃ, N, H)
                @test compare_grid_vars(getregion(grid, panel).λᶠᶠᵃ, getregion(grid_cs32, panel).λᶠᶠᵃ, N, H)

                @test compare_grid_vars(getregion(grid, panel).φᶠᶠᵃ, getregion(grid_cs32, panel).φᶠᶠᵃ, N, H)
                @test compare_grid_vars(getregion(grid, panel).λᶠᶠᵃ, getregion(grid_cs32, panel).λᶠᶠᵃ, N, H)
            end
        end
    end
end

I think it’s still useful to keep this feature for now, as it provides a benchmark reference for comparing grid metrics, especially as the codebase continues to evolve. In addition, for high-resolution simulations, especially with the non-uniform conformal mapping, we can save time on grid generation when debugging or running different simulations one after another on the same grid but with different parameters.

[glwagner]

I think it’s still useful to keep this feature for now, as it provides a benchmark reference for comparing grid metrics, especially as the codebase continues to evolve. In addition, for high-resolution simulations, especially with the non-uniform conformal mapping, we can save time on grid generation when debugging or running different simulations one after another on the same grid but with different parameters.

I agree that its useful, but the question is whether its worth it. No other grid type supports this, so why cubed sphere?

[glwagner]

If you’re seeing an error, it’s likely due to the latest merges into the main branch.

Ok, we need to work on adding more tests so that the cubed sphere does not break without us knowing

[simone-silvestri]

I agree with removing the feature of reading from file. With it also removing the CubedSphere module.

[glwagner]

@simone-silvestri we're getting an error lke

Testing simulation on conformal and immersed conformal cubed sphere grids: Error During Test at /var/lib/buildkite-agent/Oceananigans.jl-25563/test/test_multi_region_cubed_sphere.jl:744
--
  | Got exception outside of a @test
  | type Nothing has no field rank
  | Stacktrace:
  | [1] getproperty(x::Nothing, f::Symbol)
  | @ Base ./Base.jl:37
  | [2] (::Oceananigans.BoundaryConditions.MultiRegionFillHalo{SouthAndNorth})(c::OffsetArray{Float32, 3, Array{Float32, 3}}, southbc::BoundaryCondition{Oceananigans.BoundaryConditions.MultiRegionCommunication, Nothing}, northbc::BoundaryCondition{Oceananigans.BoundaryConditions.MultiRegionCommunication, Nothing}, loc::Tuple{Center, Center, Nothing}, grid::ZRegOrthogonalSphericalShellGrid{Float32, FullyConnected, FullyConnected, Bounded, Oceananigans.Grids.StaticVerticalDiscretization{OffsetVector{Float32, StepRangeLen{Float32, Float64, Float64, Int64}}, OffsetVector{Float32, StepRangeLen{Float32, Float64, Float64, Int64}}, Float32, Float32}, Oceananigans.OrthogonalSphericalShellGrids.CubedSphereConformalMapping{Rotations.RotXY{Float64}, StepRangeLen{Float32, Float64, Float64, Int64}, StepRangeLen{Float32, Float64, Float64, Int64}, StepRangeLen{Float32, Float64, Float64, Int64}, StepRangeLen{Float32, Float64, Float64, Int64}}, OffsetMatrix{Float32, Matrix{Float32}}, OffsetMatrix{Float32, Matrix{Float32}}, OffsetMatrix{Float32, Matrix{Float32}}, OffsetMatrix{Float32, Matrix{Float32}}, CPU, Float32}, buffers::Tuple{})
  | @ Oceananigans.MultiRegion ~/Oceananigans.jl-25563/src/MultiRegion/multi_region_boundary_conditions.jl:113
  | [3] fill_halo_event!
  | @ ~/Oceananigans.jl-25563/src/BoundaryConditions/fill_halo_regions.jl:40 [inlined]

which comes from

Oceananigans.jl/src/MultiRegion/multi_region_boundary_conditions.jl

Line 113 in fb6647a

southdst = buffers[southbc.condition.rank].south.recv

I'm trying to figure out where the assumption of condition.rank comes from, but it's unclear here. There is an assumption about the type of the boundary condition that isn't holding.

Note also I don't think we hit this code path on CPU. Why would we hit this on GPU but not CPU? This looks like MPI / multi-device code, but I thought we discontinued support for that.

[simone-silvestri]

yeah we should not hit that path with a CubedSphere, which should always take the paths defined in cubed_sphere_boundary_conditions.jl. Let me take a look

This looks like MPI / multi-device code, but I thought we discontinued support for that.

yeah, this is the code for a vanilla XPartition / YPartition multi-region grids, it will not work for a cubed sphere. It does not necessarily require MPI because it is all on one device, I thought I would leave this anyways to have a simple way to verify if the multi-region works.

[glwagner]

yeah we should not hit that path with a CubedSphere, which should always take the paths defined in cubed_sphere_boundary_conditions.jl. Let me take a look

This looks like MPI / multi-device code, but I thought we discontinued support for that.

yeah, this is the code for a vanilla XPartition / YPartition multi-region grids, it will not work for a cubed sphere. It does not necessarily require MPI because it is all on one device, I thought I would leave this anyways to have a simple way to verify if the multi-region works.

Hmm ok so something is going awry during setup on GPU (but not on CPU) perhaps

[glwagner]

@siddharthabishnu if you have a chance can you look into this?

[simone-silvestri]

So what was happening here is that we were calling fill_halo_regions! inside an @apply_regionally, which is not allowed. I guess from now on, when we encounter this error again, we will know what is happening.

[simone-silvestri]

Do the commented tests need to be uncommented?