Integrate cubed sphere functionality with halo filling and core infrastructure updates #4538
Conversation
siddharthabishnu
changed the title
|
could you resolve the conflicts? |
| #= | ||
| field[region][1:Nc, Nc+1:Nc+Hc, k] .= reverse(field[region_N][1:Hc, 1:Nc, k], dims=2)' | ||
| field[region][1:Nc, 1-Hc:0, k] .= field[region_S][1:Nc, Nc+1-Hc:Nc, k] | ||
| =# |
There was a problem hiding this comment.
why we have commented code here (in similar places further up and down?
do we need it?
There was a problem hiding this comment.
You're right---we don't need these commented lines. I’ve kept them because they show the equivalent non-GPU vectorized implementation, which can be useful for visually verifying halo filling against schematics or physical cubed sphere models. But if you'd prefer a cleaner file, I’m happy to remove them.
There was a problem hiding this comment.
Either delete or add a comment explaining why they are there or what they demonstrate (in the spirit of what you just explained)?
There was a problem hiding this comment.
Comments added in commit c3554fd.
src/Models/HydrostaticFreeSurfaceModels/pcg_implicit_free_surface_solver.jl
Outdated
Show resolved
Hide resolved
Co-authored-by: Navid C. Constantinou <[email protected]>
…ace_solver.jl Co-authored-by: Simone Silvestri <[email protected]>
If you're referring to the conflict that arose after fetching and merging the main branch, the only file affected was |
src/Models/HydrostaticFreeSurfaceModels/pcg_implicit_free_surface_solver.jl
Outdated
Show resolved
Hide resolved
src/Models/HydrostaticFreeSurfaceModels/pcg_implicit_free_surface_solver.jl
Outdated
Show resolved
Hide resolved
| launch!(arch, grid, :xy, _implicit_free_surface_linear_operation!, | ||
| L_ηⁿ⁺¹, grid, ηⁿ⁺¹, ∫ᶻ_Axᶠᶜᶜ, ∫ᶻ_Ayᶜᶠᶜ, g, Δt) | ||
|
|
||
| return nothing | ||
| end | ||
|
|
||
| ImplicitFreeSurfaceOperation = typeof(implicit_free_surface_linear_operation!) | ||
|
|
||
| auxiliary_actions!(::ImplicitFreeSurfaceOperation, L_ηⁿ⁺¹, ηⁿ⁺¹, args...) = fill_halo_regions!(ηⁿ⁺¹) |
There was a problem hiding this comment.
This is used in the solver module right? I am not sure this is very clear. Is there a way to maybe write it in a more streamlined way? Like, for example, by removing the @apply_regionally from line 202 of conjugate_gradient_solver.jl and write a function which in the body:
@inline function mylinearoperation(...)
@apply_regionally my_actual_linear_operation(...)
fill_halo_regions!()
endThere was a problem hiding this comment.
Please see the modifications here: f324fa7...4c4f53e.
| auxiliary_actions!(solver.linear_operation!, q, p, args...) | ||
|
|
||
| @apply_regionally solver.linear_operation!(q, p, args...) | ||
|
|
There was a problem hiding this comment.
see my comment in the Module section
| auxiliary_actions!(args...) = nothing | ||
|
|
There was a problem hiding this comment.
| auxiliary_actions!(args...) = nothing |
…sed-boundary-grid' into sb/cubed-sphere-integration
| U = Field{Face, Center, Nothing}(free_surface_grid, boundary_conditions=U_bcs) | ||
| V = Field{Center, Face, Nothing}(free_surface_grid, boundary_conditions=V_bcs) | ||
| U = Field{Face, Center, Nothing}(free_surface_grid, indices = (:, :, 1:1), boundary_conditions=U_bcs) | ||
| V = Field{Center, Face, Nothing}(free_surface_grid, indices = (:, :, 1:1), boundary_conditions=V_bcs) |
There was a problem hiding this comment.
this has no effect, since the third location is already Nothing
There was a problem hiding this comment.
and might possibly be regarded as incorrect
There was a problem hiding this comment.
Without specifying indices = (:, :, 1:1), I was running into an out-of-bounds error. After investigating, I identified the root cause: I wasn’t using reduced_dimensions in the fill_halo_regions! methods tailored for the cubed sphere. Without incorporating reduced_dimensions and without the manual indices workaround, fields defined with Nothing in the third dimension on grids with multiple vertical levels would incorrectly attempt to fill halos across all levels, despite the field having none, resulting in the out-of-bounds error. This issue has now been resolved.
| # Note: because of `fill_halo_regions!` below, we cannot use `PCGImplicitFreeSurface` on a | ||
| # multi-region grid; `fill_halo_regions!` cannot be used within `@apply_regionally` | ||
| fill_halo_regions!(ηⁿ⁺¹) | ||
|
|
||
| launch!(arch, grid, :xy, _implicit_free_surface_linear_operation!, |
There was a problem hiding this comment.
| launch!(arch, grid, :xy, _implicit_free_surface_linear_operation!, | |
| @apply_regionally launch!(arch, grid, :xy, _implicit_free_surface_linear_operation!, |
This PR integrates the cubed sphere implementation into the main branch of Oceananigans. While grid generation was addressed in a previous PR (#4295, now merged), this PR focuses on accurately filling the halos of grid metrics and relevant fields---the most computationally challenging aspect of the implementation, and introduces numerous enhancements across several core components, including:
MultiRegiongrid and field infrastructureCubedSpheregrid and field infrastructureand more. The goal is to enable seamless execution of cubed sphere simulations upon merging this PR. Note that I/O functionality (e.g., output writers, checkpointing, field time series) will be addressed in a subsequent PR.