umaskCD and more #33

TillRasmussen · 2021-11-19T21:01:23Z

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers
Still a few bugs

PR checklist

…ransport files are changed in a way so that velocities are interpolated to b grid for depature point function and kept at E or N grid possible.

…nted out in vp and eap

…ransport files are changed in a way so that velocities are interpolated to b grid for depature point function and kept at E or N grid possible.

…nted out in vp and eap

Added function to fint zeta, eta and replacement pressure at U

eclare108213 · 2021-11-20T02:52:48Z

I cloned a fresh sandbox from @apcraig's branch (through #37), pulled the changes from this branch onto it, fixed the conflict, and modified the code so that it compiles. A basic smoke test with -s gridcd fails -- the velocities blow up. :( But we reached the goal of being able to push the "go" button and see what happens, and we have something to debug now!!! @TillRasmussen might have also fixed all these same things by the time he sees this -- if not, I can create a draft PR with my changes on it, if it's helpful. I'll see if the initializations in #38 help.

apcraig · 2021-11-20T03:29:14Z

With PRs to #38, but not this PR (#33), the gx3 CD test runs to completion for 10 days with debug on. No idea what the solution looks like.

eclare108213 · 2021-11-20T04:11:26Z

Merging #38 didn't help my runs, but I might not have all of Till's intended changes -- I only fixed what had been committed so far. For the gx3 grid, is the dynamic forcing being interpolated from T to the N, E cell faces? Or is JRA coming in at U points and shouldn't be shifted? Some of the grid_sys suite runs complete but the velocities are not correct. Those will be good debugging platforms.

TillRasmussen · 2021-11-20T08:41:30Z

I cloned a fresh sandbox from @apcraig's branch (through #37), pulled the changes from this branch onto it, fixed the conflict, and modified the code so that it compiles. A basic smoke test with -s gridcd fails -- the velocities blow up. :( But we reached the goal of being able to push the "go" button and see what happens, and we have something to debug now!!! @TillRasmussen might have also fixed all these same things by the time he sees this -- if not, I can create a draft PR with my changes on it, if it's helpful. I'll see if the initializations in #38 help.

Ok, thanks.
As I could see it I missed passing tarea through and adding a few use module references to ice_dyn_shared.was missing

eclare108213 · 2021-11-20T14:30:24Z

See #39
That one has all of these changes plus mine, since I pulled from your branch. Click on the hash next to my commit to see only my changes.

Only quick suite has been run.

TillRasmussen · 2021-11-20T16:00:48Z

Without having merge your changes. I can now build and run this pull request for quick_suites. This passes the quick test
@apcraig there was a out of halo issue with the creation of the umaskjCDht. The calculation of the mask is out of bound, Therefore I crehowever there is a comment that one cannot do haloupdate with logicals.
Se line 2000 in ice_grid. I will merge the two and resolve conflicts.

TillRasmussen · 2021-11-20T16:26:13Z

I will compare the two and merge tomorrow.

apcraig · 2021-11-20T17:50:17Z

Merging #38 didn't help my runs, but I might not have all of Till's intended changes -- I only fixed what had been committed so far. For the gx3 grid, is the dynamic forcing being interpolated from T to the N, E cell faces? Or is JRA coming in at U points and shouldn't be shifted? Some of the grid_sys suite runs complete but the velocities are not correct. Those will be good debugging platforms.

I believe the JRA forcing all comes in on T points just like the CESM coupling (for now).

TillRasmussen · 2021-11-20T17:53:30Z

I should note that I only ran b grid solution not cd.

apcraig · 2021-11-20T18:02:04Z

Before finalizing PR, please run both B and CD if possible. B to check bit-for-bit. CD to check that it runs (maybe even with debug flags on). Neither is absolutely required, and whether they pass or not, I can also test/debug before (or after) the PR is merged and then created fixes and/or provide feedback. Thanks!

eclare108213 · 2021-11-21T12:56:53Z

cicecore/cicedynB/infrastructure/ice_grid.F90

+         emask(:,:,iblk)    = .false.
+         do j = jlo-nghost, jhi
+         do i = ilo-nghost, ihi
+!!!!!WARNING THIS SUM is not calculating uvmCD mask on the northern and eastern halo


This is fixed in #38, and you can probably just replace this ice_grid.F90 file with that one.

TillRasmussen · 2021-11-21T17:04:43Z

I have merged 33 and 39. 33 is still binary identical. My new merge is not.
I will add differences step by step until I find where

apcraig · 2021-11-21T18:11:50Z

I'm a little confused what's happening (which is OK), but might it be a good idea to create a new branch from the head of cgridDEV (picking up #38 and others), then merging in #33 and #39 to that branch, resolving conflicts, and debugging from there? Then you could cancel #33 and #39 and create a new PR.

TillRasmussen · 2021-11-21T19:35:43Z

I did make a new based on my merge of 39.
I am currently moving file by file (From 39 to my new) but have not yet found the difference.
. It seems to be in ice_dyn_evp.F90.
I will try to glone a new head of gridDEV and see what is there

apcraig · 2021-11-21T19:48:17Z

I think the current cgridDEV head is bit-for-bit with the cice-consortium main. But I can check again. That's with intel and gnu on cheyenne. With pgi on cheyenne, it isn't. So there is a sensitivity to compiler version and we know pgi is particular tricky. You could also try running a consortium main and the cgridDEV with mods with the debug flags. This reduces the optimization and should make it more likely to be bit-for-bit. Let me know if you have questions or want me to test/debug anything in particular.

TillRasmussen · 2021-11-21T21:03:11Z

Thanks
I have pushed most of the changes to #33 from my merge.
It is still bit for bit.
I will try to get cgridDEV and make a new baseline. Just to check. My comparisons are made with quicksuite

apcraig · 2021-11-21T21:22:56Z

Sounds good. When you think it's ready, I can run some independent tests just to confirm. Let me know when you feel it's ready.

Instead an additional if else gridsystem is introduced. subroutine evp should be refactored

eclare108213 · 2021-11-22T17:57:06Z

cicecore/cicedynB/dynamics/ice_dyn_evp.F90

-                         uvel      (:,:,iblk), vvel      (:,:,iblk), &
-                         Tbu       (:,:,iblk))
+
+            if (grid_system == trim('B')) then


why is the trim around 'B'?

should have been around grid_system (if needed)
Currently not doing anything.

Please move it to grid_system. We may clean all of these up later.

eclare108213 · 2021-11-22T17:58:56Z

cicecore/cicedynB/dynamics/ice_dyn_evp.F90

-! COMING SOON!!!
-  zetax2U = c0
-  etax2U = c0
-  rep_prsU = c0


It looks like these initializations were lost in the new code modifications. Or were they moved somewhere else?

they dont need to be initialized. They are calculated in the function after as intent out.

These lines were added as a temporary fix when viscous_coeffs_and_rep_pressure_U was not called. Now it is. I think this change is correct in the big picture.

eclare108213 · 2021-11-22T18:02:09Z

cicecore/cicedynB/dynamics/ice_dyn_shared.F90

+        zetax2T_00,zetax2T_10,zetax2T_11,zetax2T_01, &
+         etax2T_00, etax2T_10, etax2T_11, etax2T_01,  & ! 2 x visous coeffs, replacement pressure
+         maskT_00, maskT_10, maskT_11, maskT_01, &
+         tarea_00, tarea_10, tarea_11, tarea_01, &


please comment the code to explain what 00, 10, 01, 11 are

will do. These are i+0,j+0,i+1 and j+1

apcraig · 2021-11-22T18:46:15Z

cicecore/cicedynB/dynamics/ice_dyn_shared.F90

@@ -1899,7 +1903,7 @@ subroutine viscous_coeffs_and_rep_pressure (strength,  tinyarea, &
      real (kind=dbl_kind), intent(in)::  &  
        Deltane, Deltanw, Deltasw, Deltase  ! Delta at each corner

-      logical , intent(in):: capping
+      real(kind=dbl_kind) , intent(in):: capping


It looks like the intent here is that capping is either c0 or c1 as if it were a logical but by setting it to a real it avoids an if check. That should be documented in the code and should there be a check added that capping must be either c0 or c1 valued?

capping was introduced by @JFLemieux73 when he reorganized the EVP solver, This introduced an if statement within the viscous_coeffs_and_rep_pressure function. To avoid the if statement I replaced it with a c0/c1. It is c1 in the evp and c0 in the vp and set as parameters further up. Currently hard coded. The intent is to replace it with a namelist parameter both in main and in the cd grid code. This should be a string perhaps "smooth_cap" and "hard_cap" which is then turned into real (c0 and c1)

Will it always be either 0 or 1? And will it always be 1 for EVP (and presumably the variants) and 0 for VP? If so, could it be set based on kdyn at initialization, instead of namelist?

apcraig · 2021-11-22T23:08:18Z

I ran a full test suite on cheyenne (all B configurations) with cgridDEV plus this PR and all results are bit-for-bit. So I think the only issue is that the CD cases don't complete (not surprising) and whether we want to do anything more to this PR (outside maybe some cleanup) before merging and continuing to debug the CD configuration.

eclare108213

This can be merged once Till is happy with it. I've requested a few changes.

eclare108213 · 2021-11-23T16:42:21Z

cicecore/cicedynB/dynamics/ice_dyn_evp.F90

-                         uvel      (:,:,iblk), vvel      (:,:,iblk), &
-                         Tbu       (:,:,iblk))
+
+            if (grid_system == trim('B')) then


Please move it to grid_system. We may clean all of these up later.

eclare108213 · 2021-11-23T16:42:40Z

cicecore/cicedynB/dynamics/ice_dyn_evp.F90

-! COMING SOON!!!
-  zetax2U = c0
-  etax2U = c0
-  rep_prsU = c0


eclare108213 · 2021-11-23T16:44:32Z

cicecore/cicedynB/dynamics/ice_dyn_shared.F90

@@ -1899,7 +1903,7 @@ subroutine viscous_coeffs_and_rep_pressure (strength,  tinyarea, &
      real (kind=dbl_kind), intent(in)::  &  
        Deltane, Deltanw, Deltasw, Deltase  ! Delta at each corner

-      logical , intent(in):: capping
+      real(kind=dbl_kind) , intent(in):: capping


Will it always be either 0 or 1? And will it always be 1 for EVP (and presumably the variants) and 0 for VP? If so, could it be set based on kdyn at initialization, instead of namelist?

TillRasmussen · 2021-11-23T16:58:16Z

Currently yes it is the case that capping for evp=c1 and capping vp =c0
@JFLemieux73 had an idea of testing both within the evp scheme. Otherwise yes it could be set in the initialization of kdyn.

TillRasmussen · 2021-11-23T20:42:49Z

I would like to have it merged. I fixed the trim('B') but I did not add comments nor did change capping nor did I add comments.

TillRasmussen · 2021-11-23T21:25:02Z

I ran the gridsys test suite. All B cases seem to run fine and are bit-for-bit. I will run the full suite next on the B grid. When CD is turned on, most tests fail. I assume it's because there are problems in the new implementation that have to be debugged. For instance, "cheyenne_intel_restart_gx3_4x2_debug_diag1_gridcd" fails at line 931 of ice_dyn_shared in step_vel at this line

uvel(i,j) = (cca*cc1 + ccb*cc2) / ab2 ! m/s

So, the question is do we merge this and consider this the starting point to debugging the CD configuration? It does not run in general. Do we comment out some of the latest calls and revert back to the pre #39 calling implementation that ran with CD then merge? That would mean the out of the box would run with CD but it would not have everything turned on. And that could be used to start debugging the full CD. Or do we want to try to get #39 working with CD before we merge?

This bug is removed if the creation of umaskCD iincludes an icemask just as in dyn_prep2 for umask (B grid)
! iceumask(i,j) = umask(i,j)
iceumask(i,j) = (umask(i,j)) .and. (aiu (i,j) > a_min) &
.and. (umass(i,j) > m_min)

phil-blain · 2021-11-23T22:01:36Z

cicecore/cicedynB/dynamics/ice_dyn_evp.F90

-                         nmassdti  (:,:,iblk), fcorN_blk  (:,:,iblk), & 
-                         nmask     (:,:,iblk),                       & 
+                         nmassdti  (:,:,iblk), fcorN_blk  (:,:,iblk),& 
+                         nmask     (:,:,iblk),                       &                      


can we avoid unnecessary whitespace changes ? this introduces false positive in 'git blame' output, if care is not taken..,.

phil-blain · 2021-11-23T22:03:48Z

cicecore/cicedynB/dynamics/ice_dyn_evp.F90

-
-         call viscous_coeffs_and_rep_pressure (strength(i,j), tinyarea(i,j),&
-                                               Deltane,       Deltanw,      &
-                                               Deltasw,       Deltase,      &
-                                               zetax2ne,      zetax2nw,     &
-                                               zetax2sw,      zetax2se,     &
-                                               etax2ne,       etax2nw,      &
-                                               etax2sw,       etax2se,      &
-                                               rep_prsne,     rep_prsnw,    &
-                                               rep_prssw,     rep_prsse,    &
-                                               capping)
+
+         call viscous_coeffs_and_rep_pressure_T (strength(i,j), tinyarea(i,j),&
+                                                 Deltane,       zetax2ne,     &
+                                                 etax2ne,       rep_prsne,    &
+                                                 capping)
+
+         call viscous_coeffs_and_rep_pressure_T (strength(i,j), tinyarea(i,j),&
+                                                 Deltanw,       zetax2nw,     &
+                                                 etax2nw,       rep_prsnw,    &
+                                                 capping)
+
+         call viscous_coeffs_and_rep_pressure_T (strength(i,j), tinyarea(i,j),&
+                                                 Deltasw,       zetax2sw,     &
+                                                 etax2sw,       rep_prssw,    &
+                                                 capping)
+
+         call viscous_coeffs_and_rep_pressure_T (strength(i,j), tinyarea(i,j),&
+                                                 Deltase,       zetax2se,     &
+                                                 etax2se,       rep_prsse,    &
+                                                 capping)
+


That is not strictly necessary, right ? it's just cleaning up the B-grid implementation. So maybe it could be done in a later step ? If it's bit for bit, then all for the better and I guess it does not matter much...

phil-blain · 2021-11-23T22:04:27Z

cicecore/cicedynB/dynamics/ice_dyn_evp.F90

-                             epm,  npm,  uvm,       &
+                             epm,  npm, hm, uvm,    &


I think it would be preferable to restrict ourselves to 2 arguments per line, for better readability.

phil-blain · 2021-11-23T22:06:32Z

cicecore/cicedynB/dynamics/ice_dyn_shared.F90

+         ! ice extent mask (U-cells)
+            iceumask(i,j) = (umask(i,j)) .and. (aiu  (i,j) > a_min) & 
+                                         .and. (umass(i,j) > m_min)
+         else  ! ice mask shpuld be applied to cd grid. For now it is not implemented.


I think we agreed on spelling out elseif ( trim(grid_sytem) == 'CD') (or using a select statement instead). It's also clearer when we read the code, I think.

Obsolete. see soon upcoming push that make cd grid run complete

phil-blain · 2021-11-23T22:09:39Z

cicecore/cicedynB/infrastructure/ice_grid.F90

@@ -155,7 +155,7 @@ module ice_grid
      real (kind=dbl_kind), dimension (:,:,:), allocatable, public :: &
         hm     , & ! land/boundary mask, thickness (T-cell)
         bm     , & ! task/block id
-         uvm    , & ! land/boundary mask, velocity (U-cell)
+         uvm    , & ! land/boundary mask, velocity (U-cell) - land if one point is one


I'm not sure about that comment means exactly .
Also below in alloc_grid you used a different wording: "water in case of all water point"

For concistency. Comment is removed

phil-blain · 2021-11-23T22:11:23Z

cicecore/cicedynB/infrastructure/ice_grid.F90

+            if (uvm(i,j,iblk)   > p5  ) umask  (i,j,iblk)   = .true.
+            if (uvmCD(i,j,iblk) > c1p5) umaskCD(i,j,iblk) = .true.


alignment:

Suggested change

if (uvm(i,j,iblk) > p5 ) umask (i,j,iblk) = .true.

if (uvmCD(i,j,iblk) > c1p5) umaskCD(i,j,iblk) = .true.

if (uvm(i,j,iblk) > p5 ) umask (i,j,iblk) = .true.

if (uvmCD(i,j,iblk) > c1p5) umaskCD(i,j,iblk) = .true.

phil-blain · 2021-11-23T22:13:47Z

cicecore/cicedynB/infrastructure/ice_grid.F90

+      enddo
+      !$OMP END PARALLEL DO
+
+      !$OMP PARALLEL DO PRIVATE(iblk,i,j,ilo,ihi,jlo,jhi,this_block)
+      do iblk = 1, nblocks


Any reason why we are creating a second loop here ?

i and j loops are slightly different, iblk loop could be merged.

phil-blain · 2021-11-23T22:24:04Z

cicecore/cicedynB/dynamics/ice_dyn_evp.F90

+                            indxui      (:,iblk), indxuj      (:,iblk), &
+                            aiu       (:,:,iblk), umass     (:,:,iblk), &
+                            umassdti  (:,:,iblk), fcor_blk  (:,:,iblk), &
+                            umaskCD   (:,:,iblk),                       &


It's unclear to me why we need yet another call to dyn_prep2 here. I thought that all CD grid quantities were being dealt with in the two calls to dyn_prep2 below (for 'E' and 'N' quantities, line 440 and 473 in this version). Here your are adding a new call, identical ( as far as I can tell) to the B-grid case, apart from this new umaskCD array instead of umask... But on the CD grid, we are not using most of these arrays that are sent as arguments here...

umaskCD is defined different, icellu will be different (stress_u). It can be that calling dyn_prep2 is too much and a call to a simpler function could be used.

apcraig · 2021-11-23T23:09:49Z

@TillRasmussen, if you are making changes, they are not showing up in the PR. You will need to push to the cgrid-something branch. No rush, just wanted to point that out. Thanks!

…ails on 2 test. 1 is pending. Comments from Phillipe also included

…cgrid-something

TillRasmussen · 2021-11-24T01:00:58Z

small correction to cd grid. One run failed to finish when including the divergence stress #34. Otherwize no more corrections from here

eclare108213

I think we should go ahead and merge this as is, so that we all have a common code to debug. There will be a lot of cleanup on the new code before we merge it into the main repo, but for now we can put these things on our cleanup list. From this PR, these are

comment the code to explain that 00, 10, 01, 11 are i+0,j+0,i+1 and j+1
possibly define capping based on kdyn
join iblk loops in ice_grid.F90 (and review loop structure everywhere)
white space, alignments, comments, etc

JFLemieux73 · 2021-11-24T14:41:27Z

I would not define capping based on kdyn. There should be an additional input in the namelist. For example I would like to have the possibility to have capping=true or false for the implicit solver.

Sorry I am a bit busy this week and cannot help for the debugging. I should be able to get back to this next week.

eclare108213 · 2021-11-24T14:49:24Z

ok, thanks @JFLemieux73

apcraig · 2021-11-24T16:45:49Z

I am running the gridsys test suite quickly and then will merge. I believe the CD cases are now running to completion, so that's great. This should be an excellent place for us to start testing/debugging the results.

I have also added @eclare108213's bullets to CICE-Consortium#660.

apcraig · 2021-11-24T16:57:48Z

The test suite looks OK. The only problem is that the boxislands cases are aborting when CD is set. A sample traceback is

(abort_ice)ABORTED:
(abort_ice) error = (ice_write_hist)ERROR: writing variable uvelE
MPT: #6 0x000000000053a624 in ice_exit_mp_abort_ice_ ()
MPT: #7 0x00000000007755ab in ice_history_write_mp_ice_write_hist_ ()

I wonder if there are invalid values in that array or something like it. We should debug this case further as we move forward. The equivalent symmetry tests without boxislands run. The gridsys results for de8f6e9 are

https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#de8f6e973526789f7202e58ac02199ddee3e0f52

I will merge this and #40 now and then fire up a full test suite on cheyenne.

dabail10 · 2021-11-24T17:02:49Z

You can assign me to it.

dabail10 · 2021-11-24T17:24:09Z

I changed diagfreq to 1 and it now aborts on the first step in the diagnostics. The speed is blowing up here at least at one point. I'll keep looking.

Dave

total ice area (km^2) = 2.64780799999989523E+06 0.00000000000000000E+00
total ice extent(km^2) = 2.64780800000000000E+06 0.00000000000000000E+00
total ice volume (m^3) = 6.65130161482975098E+12 0.00000000000000000E+00
total snw volume (m^3) = 5.04204992333457886E+11 0.00000000000000000E+00
tot kinetic energy (J) = 3.21172820850853413+150 0.00000000000000000E+00
rms ice speed (m/s) = ************************ 0.00000000000000000
forrtl: error (63): output conversion error, unit 6, file /proc/29294/fd/1
Image PC Routine Line Source
cice 000000000216E08B Unknown Unknown Unknown
cice 00000000021C0DB6 Unknown Unknown Unknown
cice 0000000000640EB4 ice_diagnostics_m 974 ice_diagnostics.F90
cice 0000000000413CF2 cice_runmod_mp_ic 378 CICE_RunMod.F90
cice 0000000000411661 cice_runmod_mp_ci 83 CICE_RunMod.F90
cice 0000000000404CA6 MAIN__ 49 CICE.F90
cice 0000000000404C22 Unknown Unknown Unknown
libc-2.22.so 00002B2EF4A996E5 __libc_start_main Unknown Unknown
cice 0000000000404B29 Unknown Unknown Unknown

dabail10 · 2021-11-24T20:45:50Z

Quick update. Looks like the internal stresses are blowing up. This is a print inside step_vel.

aiu 0.999999999999923
uocn 0.000000000000000E+000
vocn 0.000000000000000E+000
waterx 0.000000000000000E+000
watery 0.000000000000000E+000
strintx 1.037775857340548E+047
strinty 2.229817867983094E+043
forcex 3.900000000000000E-002
forcey 0.000000000000000E+000
fm 0.345486590918712
Tb 0.000000000000000E+000

JFLemieux73 · 2021-11-24T21:25:35Z

Thanks Dave. Is it strintx,y that make uvel,vvel blow up or the opposite? Could you please print strintx,y and uvelE,vvelE at the beginning of the 1st subcycle?

dabail10 · 2021-11-24T23:09:20Z

I agree there is a bit of a chicken or egg here. Here are some print statements after stressT and stressU
ksub 3
uvelE,vvelE 1 4.944240786664977E-004 0.160903474339204
uvelN,vvelN 1 4.944240786664977E-004 2.092852846194380E-002
stresspT(:,:,iblk) 1 0.000000000000000E+000
stressmT(:,:,iblk) 1 0.000000000000000E+000
stress12T(:,:,iblk) 1 164.095560225434
uvelE,vvelE 1 4.944240786664977E-004 0.160903474339204
uvelN,vvelN 1 4.944240786664977E-004 2.092852846194380E-002
stresspU(:,:,iblk) 1 0.000000000000000E+000
stressmU(:,:,iblk) 1 0.000000000000000E+000
stress12U(:,:,iblk) 1 1639176.08411872
ksub 4
uvelE,vvelE 1 1.26759587715081 87.9467999636907
uvelN,vvelN 1 1463.14831216310 1.27697191412802
stresspT(:,:,iblk) 1 0.000000000000000E+000
stressmT(:,:,iblk) 1 0.000000000000000E+000
stress12T(:,:,iblk) 1 289.044717507070
stresspU(:,:,iblk) 1 0.000000000000000E+000
stressmU(:,:,iblk) 1 0.000000000000000E+000
stress12U(:,:,iblk) 1 196487370.229011

It does appear that stress12U goes pretty big on the third subcycle. Then the velocities at the fourth subcycle go wacky. I believe the stress12U components are going bad first ... or maybe not. Is a stress value of 1639176.08411872 too big?

Dave

JFLemieux73 · 2021-11-24T23:54:50Z

Well...the fact that stresspT and stressmT are zero is weird. There is something wrong there.

I suggest to use kstrength=0 for debugging (it is easier to interpret the values of stresses). With kstrength = 0, the stresses have values on the order of Pstar (default Pstar is 27500). We should expect values between 0 and 10*Pstar.

I will help you as soon as I can with the debugging. What test case are you running? gx3?

dabail10 · 2021-11-25T00:22:45Z

This is a boxislande case. It is a new case with a limited area mask. I am not that familiar with all the settings.

apcraig · 2021-11-25T05:38:25Z

Full test results on cheyenne with 3 compilers is here,

https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#9e6e8f330efbadb7d1613862e32c3b8c5cebfa8b

The test suites with 232 cases are the standard full suites compared on the B grid with Consortium main. The test suites with 28 tests are the gridsys suite which consists of B and CD tests and those are compared to the last cgridDEV version (which is pretty meaningless at this point).

Everything looks pretty good. B configurations are still bit-for-bit with Consortium main except pgi and some box tests that have changes. CD cases generally run except the case with boxislands. The pgi non-bit-for-bit is expected. The boxislands will require a bit more debugging, but that seems to be underway. And again, this does not suggest any of the CD results are reasonable, only that the short CD test runs completed.

eclare108213 · 2021-11-25T13:35:27Z

Here are some questions that might provide clues. Maybe you've already looked at these things.

Are there different namelist settings, other than the land mask, between the run being debugged and a similar one that completes (e.g. symmetry)? If so, does changing any of those settings help, or is the problem really related to the land mask? The options settings are a little confusing -- we have boxislands tests and also a kmtislands option applied to the symmetry tests. Based on comments farther up, I think this is a test that uses the spatially variable forcing from the box2001 tests. Is that right? And the symmetry tests with this land mask run to completion? Does running one of the symmetry test configurations with this forcing instead, also break? Or does running this test with the symmetry forcing still break?
Is this being run on multiple processors and/or multiple blocks, and if so, does it complete using -p 1x1?
@dabail10 Is the point you're looking at special, or is this happening all across the domain? If it's special, what is the block (i,j,iblk) or global (i,j) coordinate? Is it located on an edge, i.e. on a block edge, in a halo, or next to a coastline? If so, on what side(s) is the edge?

JFLemieux73 · 2021-11-25T19:44:41Z

I have started to investigate what is going on. I think there is an instability in compact regions close to land. I have created a new issue specifically for this: CICE-Consortium#666

It is issue 666...not a good sign! :)

TillRasmussen added 7 commits November 19, 2021 19:18

Changes in ice_dyn* are interpolation of uvelE/vvelN to B grid. ice_t…

cf65c39

…ransport files are changed in a way so that velocities are interpolated to b grid for depature point function and kept at E or N grid possible.

changed according to comments. changed average from F to S and. comme…

89e401a

…nted out in vp and eap

Changes in ice_dyn* are interpolation of uvelE/vvelN to B grid. ice_t…

24d98db

…ransport files are changed in a way so that velocities are interpolated to b grid for depature point function and kept at E or N grid possible.

changed according to comments. changed average from F to S and. comme…

8849523

…nted out in vp and eap

Create local uvmCD that is projected into umaskCD and icellu.

ae40964

Added function to fint zeta, eta and replacement pressure at U

Added call to routine

3855b8c

bux fixes

004aa9c

apcraig changed the title ~~Cgrid something~~ umaskCD and more Nov 19, 2021

last bugfixes. Code compiles and is bit for bit with code before.

fcd291b

Only quick suite has been run.

eclare108213 reviewed Nov 21, 2021

View reviewed changes

First move towards merge with merge of commit 39

710967f

update ice_dyn_evp

9b1ec14

TillRasmussen and others added 2 commits November 21, 2021 22:51

It is now bit for bit. Number of mask is reduced from 2 to one.

f5d7d89

Instead an additional if else gridsystem is introduced. subroutine evp should be refactored

Merge branch 'cgridDEV' into cgrid-something

15c16f5

eclare108213 reviewed Nov 22, 2021

View reviewed changes

apcraig reviewed Nov 22, 2021

View reviewed changes

eclare108213 requested changes Nov 23, 2021

View reviewed changes

moved trim

65d46c0

phil-blain reviewed Nov 23, 2021

View reviewed changes

TillRasmussen added 2 commits November 23, 2021 23:57

Based on Quicksuite: B grid bit for bit. CD grid passes 13 of 16. 2 f…

82c90b2

…ails on 2 test. 1 is pending. Comments from Phillipe also included

Merge branch 'cgrid-something' of github.com:TillRasmussen/CICE into …

de8f6e9

…cgrid-something

eclare108213 approved these changes Nov 24, 2021

View reviewed changes

apcraig merged commit c4e0204 into apcraig:cgridDEV Nov 24, 2021

TillRasmussen deleted the cgrid-something branch November 25, 2021 18:35

		if (uvm(i,j,iblk) > p5 ) umask (i,j,iblk) = .true.
		if (uvmCD(i,j,iblk) > c1p5) umaskCD(i,j,iblk) = .true.

umaskCD and more #33

umaskCD and more #33

Conversation

TillRasmussen commented Nov 19, 2021

PR checklist

eclare108213 commented Nov 20, 2021 • edited Loading

apcraig commented Nov 20, 2021

eclare108213 commented Nov 20, 2021

TillRasmussen commented Nov 20, 2021

eclare108213 commented Nov 20, 2021 • edited Loading

TillRasmussen commented Nov 20, 2021

TillRasmussen commented Nov 20, 2021

apcraig commented Nov 20, 2021

TillRasmussen commented Nov 20, 2021

apcraig commented Nov 20, 2021

Choose a reason for hiding this comment

TillRasmussen commented Nov 21, 2021

apcraig commented Nov 21, 2021

TillRasmussen commented Nov 21, 2021

apcraig commented Nov 21, 2021

TillRasmussen commented Nov 21, 2021

apcraig commented Nov 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TillRasmussen Nov 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apcraig commented Nov 22, 2021

eclare108213 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TillRasmussen commented Nov 23, 2021

TillRasmussen commented Nov 23, 2021

TillRasmussen commented Nov 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apcraig commented Nov 23, 2021

TillRasmussen commented Nov 24, 2021

eclare108213 left a comment

Choose a reason for hiding this comment

JFLemieux73 commented Nov 24, 2021

eclare108213 commented Nov 24, 2021

apcraig commented Nov 24, 2021

apcraig commented Nov 24, 2021

dabail10 commented Nov 24, 2021

dabail10 commented Nov 24, 2021

dabail10 commented Nov 24, 2021

JFLemieux73 commented Nov 24, 2021

dabail10 commented Nov 24, 2021

JFLemieux73 commented Nov 24, 2021

dabail10 commented Nov 25, 2021

apcraig commented Nov 25, 2021

eclare108213 commented Nov 25, 2021

JFLemieux73 commented Nov 25, 2021

eclare108213 commented Nov 20, 2021 •

edited

Loading

eclare108213 commented Nov 20, 2021 •

edited

Loading

TillRasmussen Nov 23, 2021 •

edited

Loading