Completely overhaul grid utils + min_Δx/y/z -> minimum_spacing + move x/y/zspacing to Grids
#2991
Conversation
navidcy
changed the title
min_Δx/y/z -> minimum_spacing + move x/y/zspacing to Gridsmin_Δx/y/z -> minimum_spacing + move x/y/zspacing to Grids
…l into ncc/miminum_spacing
|
@glwagner I think there is still an issue in regrid_xy, only on GPU now. |
|
I think we can't call |
|
omg, perhaps I fixed it... see 5272106? |
|
now it's getting stuck on equality of two cuda subarrays... could you have a look at the GPU unit tests @glwagner? |
| @@ -41,7 +41,7 @@ function Base.:(==)(grid1::AbstractGrid, grid2::AbstractGrid) | |||
| x1, y1, z1 = nodes(grid1, (Face(), Face(), Face())) | |||
| x2, y2, z2 = nodes(grid2, (Face(), Face(), Face())) | |||
|
|
|||
| return x1 == x2 && y1 == y2 && z1 == z2 | |||
| CUDA.@allowscalar return x1 == x2 && y1 == y2 && z1 == z2 | |||
There was a problem hiding this comment.
There's a nicer way to fix this --- this is sort of a hack that will be quite slow for large grids.
The nodes are using a view of an OffsetArray of a CuArray. From the stacktrace:
==(A::SubArray{Float64, 1, OffsetVector{Float64, CuArray{Float64, 1, CUDA.Mem.DeviceBuffer}}, Tuple{UnitRange{Int64}}, true}, B::SubArray{Float64, 1, OffsetVector{Float64, CuArray{Float64, 1, CUDA.Mem.DeviceBuffer}}, Tuple{UnitRange{Int64}}, true})
There's no GPU-friendly == operator for that deeply nested object. However, there are GPU-friendly operators for subarray.
I think when we return xnodes, etc it's probably nicer to use a plain view around the underlying array.
We can leave this for this PR and I can open a new PR to fix it later.
There was a problem hiding this comment.
Thanks. I was wondering yesterday about this.
|
Seems likes docs are about to fail probably due to |
I fixed everything so we are back on the 6hr30min train of building the docs.... if you wanna fix the grid equality sometime soon then go for it, otherwise let's leave it for a future PR... |
ok |
This PR overhauls grid utils + adds
minimum_x/y/zspacingmethods that work on any grid.(with @glwagner)
Closes #1119