Oceananigans.jl icon indicating copy to clipboard operation
Oceananigans.jl copied to clipboard

Perform some of the grid tests on GPU as well

Open navidcy opened this issue 2 years ago • 11 comments

In response to #3040 I make sure that some of the grid tests also happen on GPU.

navidcy avatar Apr 06 '23 07:04 navidcy

Good idea!

@navidcy did you manually cancel the tests?

tomchor avatar Apr 06 '23 23:04 tomchor

I don't think we can test right now since we ran out of buildkite minutes. We have to wait until April 10.

glwagner avatar Apr 07 '23 00:04 glwagner

Good idea!

@navidcy did you manually cancel the tests?

I did, because it was futile as @glwagner mentioned.

navidcy avatar Apr 07 '23 01:04 navidcy

@glwagner can you review? should we make more grid tests run on GPU?

navidcy avatar Apr 11 '23 04:04 navidcy

Are there any tests for xnodes with stretched grids?

glwagner avatar Apr 11 '23 16:04 glwagner

Are there any tests for xnodes with stretched grids?

There seem to be no tests for x/y/z/λ/φnodes whatsoever. Shall we add?

navidcy avatar Apr 11 '23 20:04 navidcy

Are there any tests for xnodes with stretched grids?

There seem to be no tests for x/y/z/λ/φnodes whatsoever. Shall we add?

I think at this point there are tests for x/y/znodes? If not, it's a good idea to add a couple.

In any case this PR is already an improvement, so I approved it. @navidcy I'll leave it up to you if you wanna merge as is.

tomchor avatar Jun 24 '23 18:06 tomchor

I added some more but they definitely don't cover all cases.

Either I forgot or I am missing the point of x/y/z/λ/φspacing methods (the ones that give you a particular spacing). Like where would those be useful?

Btw, a periodic grid with faces at [0, 1, 11, 101] implying centers at [0.5, 6, 56] has the following spacings. It took me a while to confirm that the first spacing for Faces is actually correct.

julia> grid = RectilinearGrid(size=3, topology=(Periodic, Flat, Flat), x=[0, 1, 11, 101])
F = vcat(F₋, interiorF, F₊) = [-101.0, -100.0, -90.0, 0.0, 1.0, 11.0, 101.0, 102.0, 112.0, 202.0]
C = [(F[i + 1] + F[i]) / 2 for i = 1:TC] = [-100.5, -95.0, -45.0, 0.5, 6.0, 56.0, 101.5, 107.0, 157.0]
3×1×1 RectilinearGrid{Float64, Periodic, Flat, Flat} on CPU with 3×0×0 halo
├── Periodic x ∈ [0.0, 101.0)     variably spaced with min(Δx)=1.0, max(Δx)=90.0
├── Flat y
└── Flat z

julia> xspacings(grid, Center())
3-element view(OffsetArray(::Vector{Float64}, -2:5), 1:3) with eltype Float64:
  1.0
 10.0
 90.0

julia> xspacings(grid, Face())
3-element view(OffsetArray(::Vector{Float64}, -3:6), 1:3) with eltype Float64:
 45.5
  5.5
 50.0

navidcy avatar Jun 25 '23 00:06 navidcy

I'm leaning towards merging as is or closing. @glwagner?

navidcy avatar Jun 25 '23 04:06 navidcy

OK, I added some tests for variably spaced grids as well. @glwagner, what do you think?

navidcy avatar Jul 01 '23 10:07 navidcy

@glwagner I'm thinking to close this. Shall I?

navidcy avatar Aug 29 '23 09:08 navidcy