Conversation
Fill halo region and compute velocities from streamfunction.
…ns.jl into ss/correct-tripolar-red
…ns.jl into ss/correct-tripolar-red
|
I think the whitespace check might be allucinating here (or I might be allucinating). I cannot find the whitespaces that it points to! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5099 +/- ##
==========================================
- Coverage 73.97% 73.84% -0.14%
==========================================
Files 400 401 +1
Lines 22870 22938 +68
==========================================
+ Hits 16918 16938 +20
- Misses 5952 6000 +48
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:
|
| valid_domain = !((i > Nx÷2) & (j == Ny)) | ||
| return ifelse(ℓy == Face, true, valid_domain) |
There was a problem hiding this comment.
| valid_domain = !((i > Nx÷2) & (j == Ny)) | |
| return ifelse(ℓy == Face, true, valid_domain) | |
| prognostic_cell = !((i > Nx÷2) & (j == Ny)) | |
| return ifelse(ℓy == Face, true, prognostic_cell) |
this condition has to be matched by something in the zipper boundary condition halo fill. Should we try to encode this information just one place? (not sure how to do that, but it seems fragile the way this is written)
There was a problem hiding this comment.
In the zipper boundary condition we do substitute the repeated row such that we do not allow precision errors to accumulate (on centers). So in practice this last half row should be identically equal to the other one.
There was a problem hiding this comment.
right I understand that. I am just commenting on the fact that this critical information --- "the last half row is identically equal to the other one" --- is encapsulated in two places: 1) filling halos and 2) here, in the definition of the prognostic cells. ideally, we only have one place where information is stored and they are not disconnected from one another, far away in different files. but it may not be possible.
There was a problem hiding this comment.
I think it might be possible if we are not dealing with half-rows (we just change interior for example), but with a half-row it might be very difficult because we lose the matrix representation.
There was a problem hiding this comment.
shall we merge this?
|
This is ready to merge @glwagner |
|
Adapted to the new changes merged in #5408. This should be ready to go @briochemc |
Clarified comments regarding reduction operations for tripolar grids and noted limitations for UPivot distributed tripolar grids.
4 participants
This PR fixes reductions on a tripolar grid to make sure that the extra repeated half-line is excluded from the reduction.
This should also solve the problem with the tripolar conservation test.
closes #4488 #4487