Shift the RightFaceFolded fold upwards by one row#5408
Shift the RightFaceFolded fold upwards by one row#5408simone-silvestri merged 28 commits intomainfrom
RightFaceFolded fold upwards by one row#5408Conversation
Remove length definitions for RightFaceFolded.
…anigans.jl into ss-bp/update-fpivot-tripolar
|
FWIW it passed the more comprehensive tests from PR #5405 locally for me! 🚀 |
…anigans.jl into ss-bp/update-fpivot-tripolar
…anigans.jl into ss-bp/update-fpivot-tripolar
|
Probably a good practice is to merge #5099 so that we have a test that exercises the dynamics next to the seam and we are sure that this PR does not change the dynamics on a RightFaceFolded tripolar grid |
…anigans.jl into ss-bp/update-fpivot-tripolar
src/OrthogonalSphericalShellGrids/right_face_folded_kernel_parameters.jl
Outdated
Show resolved
Hide resolved
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5408 +/- ##
==========================================
+ Coverage 74.00% 74.03% +0.03%
==========================================
Files 398 399 +1
Lines 22834 22830 -4
==========================================
+ Hits 16898 16903 +5
+ Misses 5936 5927 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@briochemc I opened this PR so cannot approve. If you approve though, I merge |
Removed unnecessary halo adjustment for RightFaceFolded topology.
briochemc
left a comment
briochemc
left a comment
There was a problem hiding this comment.
I'm happy to merge this as I think it's cleaner than what we had before. I'm also eager to move on to the distributed version because I can't seem to get any speedup on my Claude-bloated PR with extra MPI passes 😅
4 participants
Given the difficulties of #5381 which probably require one extra MPI pass we want to avoid, we have decided to revisit the definition of a
RightFaceFoldedtripolar grid by shifting upwards by one row the tripolar fold. This has a couple of consequences:Centered in y have a nice and well-behaved prognostic domain that goes from1:Nywhere all the halos are filled by the fold BCFaces are a bit more problematic since the prognostic domain extends up toNy+1, where theNy+1row mirrors theCenterNyrow for aRightCenterFoldedtripolar grid, where half is prognostic and half is repeated. With this fold location we can leverage the symmetry with theUPivotboundary condition. The main downside is the need to extend all the kernels toNy + 1given that the prognostic domain extends one-row up. For all other purposes, theRightFaceFoldedtopology switches from behaving as aPeriodic/FullyConnected/RightConnected/RightCenterFoldedtopology to behaving as aBounded/LeftConnectedtopology, where theFacefields have one extra point than theCenterfields.For this reason this PR introduces a
grid_worksize, which is the size of the grid used for kernel launching (I am up to changing the name). This defaults tosize(grid)for all grids except for a right-face-folder tripolar grids where it returns Ny+1 at the second spot.cc @briochemc
Adding these
TripolarGridvisualisations fromvalidation/, made with the code in this PR:Also adding this visualisation taken from PR #5405 below (not merged yet so I copied the julia code, updated it, and ran it). If anything was off the half circles would not be centred around the pivot points, so LGTM!