Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C-Grid release tasks #870

Closed
2 of 9 tasks
apcraig opened this issue Sep 17, 2023 · 28 comments
Closed
2 of 9 tasks

C-Grid release tasks #870

apcraig opened this issue Sep 17, 2023 · 28 comments

Comments

@apcraig
Copy link
Contributor

apcraig commented Sep 17, 2023

This is a list of "todo" for the next CICE release, related to the C-Grid tasks.

Please edit the list and add items as needed. Please tick off if complete. Feel free to consolidate other open C-grid issues here with a link and close the other issue if appropriate.

  • Tony resurrect the “boxislands” test or create a new one with 1-grid-cell-wide channels
  • Tony or JF create a tx1 test with C-grid turned on
  • Philippe or JF test the C-grid code on ECCC’s tripoleT grid
  • David H test the C-grid code on NRL’s tripoleT grid
  • Dave B test the C-grid code in CESM (regular tripole grid)
  • JF submit a PR with the current code and updated documentation
  • JF review C-grid github issues, close/consolidate as needed
  • JF or Dave B provide B and C grid results for release, see provide C-grid results to community as part of next release #741
  • JF and Elizabeth Review release documentation
@phil-blain
Copy link
Member

  Philippe or JF test the C-grid code on ECCC’s tripoleT grid

That would be for later, I don't think we want to delay the next CICE release for this. This implies code changes to the ocean/ice coupling, so it's not a simple task.

@daveh150
Copy link
Contributor

daveh150 commented Sep 18, 2023 via email

@apcraig
Copy link
Contributor Author

apcraig commented Sep 18, 2023

@daveh150, there is already a tripole grid in the existing test suite, like

configuration/scripts/tests/base_suite.ts:
restart tx1 40x4 dsectrobin,medium

You should be able to setup a B-grid and C-grid test quickly for tripole with something like to provide some initial results for comparison.

./cice.setup --test smoke -g tx1 -p 80x1 -s droundrobin -m xxx -e yyy --testid zzz
./cice.setup --test smoke -g tx1 -p 80x1 -s droundrobin,gridc -m xxx -e yyy --testid zzz

I was going to give this a try myself, but happy for you to take the lead. Thanks!

@apcraig
Copy link
Contributor Author

apcraig commented Sep 18, 2023

The box islands tests with B grid and C grid shows no flow for either after 10 days in the 1 gridcell wide channels. The B grid velocity is 2x smaller in general with significant other difference. And the gridcells with no velocity are the same. That seems odd. The two cases I ran were

cheyenne_intel_smoke_gbox80_4x2_boxforcee_boxopen_kmtislands_run10day
cheyenne_intel_smoke_gbox80_4x2_boxforcee_boxopen_gridc_kmtislands_run10day

Anyone else want to confirm?

Adding plots of uvel after 10 days, white areas are "land". black areas are "zero velocity". I think there is a little error in the plotting package in terms of the exact location of the black boxes, seems to be shifted a tiny bit down and right, please ignore the tiny white spaces.

B grid:
uvelb0

C grid:
uvelc0

@apcraig
Copy link
Contributor Author

apcraig commented Sep 18, 2023

@JFLemieux73, do we need to change the default settings for C grid tests? How are you setting up your C-grid tests and in particular, the one gridcell tests? Are you manually changing namelist? Maybe that's why the default C-grid results for kmtislands above isn't working as expected?

@daveh150
Copy link
Contributor

daveh150 commented Sep 18, 2023 via email

@JFLemieux73
Copy link
Contributor

@apcraig have a look at this (closed) issue:

#699

This is a very stiff problem. The EVP solver has difficulties to converge. You could decrease Pstar (e.g. Pstar = 0) if you really want to verify you have transport in the small channel. Also, make sure you have ktransport=kridge=1.

@JFLemieux73
Copy link
Contributor

By the way here is my branch with the code for the one grid cell wide channels:

https://github.com/JFLemieux73/CICE/tree/OneGridChannels

@apcraig
Copy link
Contributor Author

apcraig commented Sep 18, 2023

OK, I'll setup the tests. If we need code or namelist modifications for one grid cell wide channel flow, we either need to make sure it's all controllable by namelist or we might as well not set it up and test it. I see the code changes in the OneGridChannels branch above. Do those change results outside the one grid channels? If not, why don't we just have that on all the time? Also, is it just

ktransport=1
kridge=1

what about pstar, you say "could". We need to pick something to use for the test. What should we set pstar to? Any other changes we need to set for the one channel test?

Also, we are testing the kmtislands test at the moment. Should we modify those tests in the same way?

I'm confused about the single gridcell channels. How is it useful if they only work in very specific setups that are not scientifically useful for real problems?

@apcraig
Copy link
Contributor Author

apcraig commented Sep 18, 2023

I just talked to @JFLemieux73. We have a plan. We need some minor code modification to trigger the remap advection in one grid channels. I will put those mods into a branch and then create a few tests, both east-west and north-south channels with thermo off. Will also look at the kmt islands cases once the single gridcell channels are working.

@JFLemieux73
Copy link
Contributor

Maybe a simple way to verify if there is transport is to look at f_dvidtd. It should .ne. 0 for the C-grid and = 0 for the B-grid.

@phil-blain
Copy link
Member

I ran two 5-years simulations on the tx1 grid, one with the B grid and one with the C grid, with the JRA55do forcing, on the latest main. Both completed successfully and gave very similar results.

@apcraig
Copy link
Contributor Author

apcraig commented Sep 25, 2023

I have single gridcell channels setup and working. Will have a PR soon.

@apcraig
Copy link
Contributor Author

apcraig commented Sep 26, 2023

Testing with the single channel modifications, https://github.com/JFLemieux73/CICE/tree/OneGridChannels, changes answers for 80% of the C and CD grid test cases. @JFLemieux73, is that expected? B cases are bit-for-bit. Those C and CD test cases are gx3 and gx1 test cases. The gbox test cases are bit-for-bit. Does this suggest that gx3 and gx1 have some "single grid channel" regions?

@JFLemieux73
Copy link
Contributor

Hi Tony,

This is surprising. Indeed this would mean there are single grid channels in gx3 and gx1 (if what I coded is right).

@JFLemieux73
Copy link
Contributor

I had a quick look at gx3. I think it is only one cell that causes the difference. Look at James Bay (South of Hudson Bay). It is the only ocean cell in the domain that is surrounded by 3 land cells. The North edge has a vvelN calculated. This is why there is a transport that could be calculated for this edge.

I think this is fine and this explains that gx3 is not BFB. We need to do the QC but use a C-grid simulation for the reference (main code) and another C-grid simulation for the test (new code...this branch).

@apcraig
Copy link
Contributor Author

apcraig commented Sep 26, 2023

The other strange thing is that I'm getting channel flow WITHOUT the latest code mods in the kmtislands case. I'm trying to understand that. I wasn't getting it in earlier testing. Something is odd, debugging now.

@JFLemieux73
Copy link
Contributor

Is it possible the kmtislands has uvel,vvel .ne. 0 at land-ocean boundaries? That would explain why you get flow without the latest mods.

@JFLemieux73
Copy link
Contributor

I just looked at gx1. There are 3 cases that could cause the non BFB differences. Again they are not really channels. An example is a one grid cell island that is separated from the main land by only one ocean cell. It makes sense to get a flow there with the C-grid. Again this looks good but would require the QC test.

@apcraig
Copy link
Contributor Author

apcraig commented Sep 26, 2023

After some additional testing, this is my conclusion. The prior kmtislands results (above) were incorrectly plotting uvel. For the C-grid, we need to plot uvele. So, running a bunch of clean tests; without @JFLemieux73 modifications in locate_triangles, we DO generate velocities both for kmtislands and for single grid channels. Rerunning those cases with the modifications produces identical bit-for-bit results on those "gbox" tests.

However, @JFLemieux73 modifications in locate_triangles DOES affect the solutions for gx3 and gx1. I guess some "extra" gridcells are being triggered in those cases as @JFLemieux73 suggests.

For the B-grid cases, the modifications play no role and there is no velocity in single gridcell channels.

So, the question is. Do we want to commit @JFLemieux73 modifications to locate_triangles? The single gridcell channels are unaffected by those modifications, but gx3/gx1 on the C-grid are. Thoughts?

@apcraig
Copy link
Contributor Author

apcraig commented Sep 26, 2023

Also, happy to do some QC tests if we want to keep the modifications. I think we can assume QC will pass. Before I do that testing, I guess I'm asking whether we want to abandon that modification or not. Maybe it's useful for generating velocity when a gridcell is blocked on three sides? Is that good? Although why it doesn't affect the kmtislands testing is still a bit of a mystery to me, maybe we have some physics/dynamics turned off in those cases that avoid that part of the code.

@apcraig
Copy link
Contributor Author

apcraig commented Sep 26, 2023

The box channel cases use default physics/dynamics settings plus ktherm = -1.

The kmtislands cases use

ktherm         = -1
kstrength      = 0
kdyn           = 1
kridge         = -1
ktransport     = -1

@apcraig
Copy link
Contributor Author

apcraig commented Sep 26, 2023

I think that explains why kmtislands and channel cases are bit-for-bit with the locate_triangles modifications. The channel case has only thermo off, but produces flow even without the modifications. The kmtislands case also produces flow, but since kridge=ktransport=-1, it's not picking up the remap modifications in locations where there might be flow closed in on 3 sides.

@JFLemieux73
Copy link
Contributor

Thanks Tony. We do need the modifications. When testing the single grid channels we want ktransport=kridge=1. I would do the same for kmtislands. The velocities (uvelE, vvelN) as calculated by the evp might be non zero but it does not mean that remapping would calculate a transport. With the current main code, the transport is zero. With the new code, there is transport in one grid cell wide channels. You could output the field f_dvidtd. If it is non zero then there is transport.

For gx3-gx1, it also makes sense to calculate transport in the cases I described above. Consider the following example (O=ocean cell, X=land cell):

OOO
XOX
XXX

If you consider the middle cell (an ocean cell), the transport at the northern edge should be calculated (which you only get with the new code)

So again I think the modifications make sense.

@apcraig
Copy link
Contributor Author

apcraig commented Sep 26, 2023

OK, I'm seeing that now. The channel case is not identical, just that both with and without the modifications, there is uvelE. But without the modifications, dvidtd is 15 orders of magnitude smaller (note that it's not zero). Will include the modifications and do a QC test as well.

@dabail10
Copy link
Contributor

Do we have a version of the code that I should be testing with CESM / tripole grids? Just from Jean-François' branch?

@apcraig
Copy link
Contributor Author

apcraig commented Sep 27, 2023

@dabail10. I recommend using the current main plus #875. #875 may be merged in the next day or so as well.

@apcraig
Copy link
Contributor Author

apcraig commented Feb 26, 2024

I think I'll close this now since the C-grid has been released. We can start new issues as problems arise.

@apcraig apcraig closed this as completed Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants