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.

Sign up for GitHub

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

Jump to bottom

Implement box model test from 2001 JCP paper #151

Merged
merged 36 commits into from
Oct 22, 2018
Merged

Implement box model test from 2001 JCP paper #151

merged 36 commits into from
Oct 22, 2018

Conversation

mattdturner
Copy link
Contributor

@mattdturner mattdturner commented Jun 20, 2018 •
edited
Loading

This PR implements the box model test defined in Hunke, E.C., 2001. Viscout-plastic sea ice dynamics with the EVP model: linearization issues. J. Comp. Phys. 170, 18-38

  • Developer(s): Matt Turner, Rick Allard

  • Please suggest code Pull Request reviewers in the column at right.

  • Are the code changes bit for bit, different at roundoff level, or more substantial? bfb

  • Is the documentation being updated with this PR? (Y/N) N
    If not, does the documentation need to be updated separately at a later time? (Y/N) Y
    Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
    which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.

  • Other Relevant Details:

  • Majority of work performed by @rallard77

  • To run the test ./cice.setup -m conrad --test smoke -s box2001 --testid t00 --grid gbox80

@mattdturner
Copy link
Contributor Author

Unless I'm mistaken, modifications to ice_init.F90 and ice_grid.F90 will likely need to be made prior to merging. Right now, it looks like some values were changed in order for this case to run (although I could be mistaken).

@rallard77
Copy link
Contributor

rallard77 commented Jun 20, 2018 via email

apcraig
apcraig reviewed Jun 24, 2018
View reviewed changes
Copy link
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there things outstanding still to do, like setting the grid box size or the coriolis setup? I agree it makes sense to add a couple namelist to define dx and dy for the box grids. We will probably need/want a few different grids for the various cases.

@rallard77
Copy link
Contributor

rallard77 commented Jun 25, 2018 via email

@mattdturner
Copy link
Contributor Author

I just pushed modifications that include a constant coriolis (included as a namelist option), and there are also namelist settings to define dx and dy.

eclare108213
eclare108213 requested changes Jul 30, 2018
View reviewed changes
Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it's taken so long for me to look at this.

I think dxrect,dyrect should always be user specified, and I recommend making those the namelist entries rather than x_spacing,y_spacing with the grid_spacing flag.

Also, these new namelist variables need to be added to the documentation along with a description of the new configuration and tests. @duvivier can help locate the right places in the docs, keeping in mind that we will have a variety of tests using the box configuration. This documentation could be put in a separate PR, but we at least need to document the namelist changes.

@mattdturner
Copy link
Contributor Author

This PR has been updated to have dxrect and dyrect in the namelist instead of x_spacing and y_spacing.

Documentation has not yet been addressed

@apcraig
Copy link
Contributor

apcraig commented Aug 2, 2018

Is there a run that validates against the original results?

@mattdturner
Copy link
Contributor Author

@apcraig , not yet. I run the full base_suite to make sure these changes didn't introduce new errors, but I can go ahead and generate a baseline and compare this PR to the baseline. Do you think I should make the baseline from master or from cice6.0?

@rallard77
Copy link
Contributor

rallard77 commented Aug 3, 2018 via email

eclare108213
eclare108213 requested changes Aug 3, 2018
View reviewed changes
Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this better. The box_data subroutine and initialization needs some more work to implement the symmetry test, but let's put that in a separate PR. Please add dxrect, dyrect and coriolis to the documentation. What's the purpose of set_nml.gbox80? Those parameters are included in set_nml.box2001. I'll run the code to see if it does what I expect.

@mattdturner
Copy link
Contributor Author

I included set_nml.gbox80 so that users could use the gbox80 grid without needing to use the box2001 test. So, users could use the gbox80 configuration to build their own test.

@eclare108213
Copy link
Contributor

The code runs but isn't configured correctly. There are several issues:

  • It looks like block boundaries aren't being handled correctly. I would guess that this is in the initialization.
  • There should be land cells around the entire domain, not just the 2 corners.
  • The 2001 test is for dynamics only, so ice area and thickness should remain constant. That requires turning off the thermodynamics and the ridging. We can create additional tests with those things turned on, once we get this basic one working.
  • The ocean currents do not have the correct pattern (should be circular). I'm not sure if the atmo vectors are correct, they are at least pointed in roughly the right direction but also seem to have the block-boundary problem.

aice
uvel
ocncurrent
wind

@rallard77
Copy link
Contributor

rallard77 commented Aug 3, 2018 via email

@eclare108213 eclare108213 mentioned this pull request Aug 8, 2018
Closed
@rallard77
Copy link
Contributor

We changed the code to compile on only one processor and set nproc=1. The currents now look reasonable. See image below.
Set kdyn=0, thickness does not change but aice is still changing. The "edeco" code specifes open boundaries for "ew_boundary_type" and "ns_boundary_type. Please advise.
currents_1x1

@eclare108213
Copy link
Contributor

kdyn=0 defeats the purpose of the 2001 test -- it turns the dynamics off and leaves the thermo on. We need to turn the thermo off and have the dynamics on. Turning the thermo off is a little complicated -- there is not a namelist flag to do that. It will need to be added. However, the wind stress needed for the dynamics is calculated as part of the thermodynamics. In the past I've probably handled that by specifying the wind stress directly, as a forcing.

The code distinguishes between open (or cyclic) boundaries, which can allow movement across the boundaries if there is no land in the way, and the land mask, which is used to close the boundaries. For the box tests, I usually specify open boundaries and then define the land mask as desired. For the 2001 case we want land cells all the way around the domain. For other tests, e.g. exercising the restoring options, we'll want some of the boundary cells to not be land. Does that make sense?

We may want to configure these tests for more than one processor, eventually. I guess that doesn't work because the analytical forcing functions and initial state should be defined globally, and then they need to be broadcast to the processors and blocks.

@rallard77
Copy link
Contributor

rallard77 commented Aug 8, 2018 via email

@duvivier
Copy link
Contributor

duvivier commented Sep 6, 2018

@mattdturner
The location of the documentation to which you will need to add the namelist info is:
/doc/source/user_guide/ug_case_settings.rst
Search the file for the table of namelist options. Add dxrect and dyrect to the grid_nml section and coriolis to the domain_nml section.

I assume there will also be some more detailed documentation as well about the box model test and how to implement it. I think this should go in this file:
/doc/source/user_guide/ug_testing.rst

When you get these files started I can help with making sure the references are done correctly, etc. We already reference the Hunke 2001 paper, so we will just need to make sure you use the correct reference when describing the test.

@eclare108213
Copy link
Contributor

I think this PR is ready to be merged. I'm waiting for 2 tests to complete in the base_suite then will post a link to those results. Here are plots from the output at day 10, a selection of which I suggest we post on the wiki as part of the release, on a "sample output" page as we have in Icepack.

ocncurrent

wind

strair

icevec

uvel

vvel

divu

ellipse

@eclare108213 eclare108213 mentioned this pull request Oct 21, 2018
Closed
4 tasks
@eclare108213
Copy link
Contributor

base_suite results:
https://github.com/CICE-Consortium/Test-Results/wiki/2689ed4109.pinto.intel.181021.130806

The 2 failing tests are because pinto does not have 40 processors available for those tests.

apcraig
apcraig approved these changes Oct 21, 2018
View reviewed changes
Copy link
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks fine. I have added one comment about some code that should be removed, but not a big deal. I think this can be merged. I assume it's up to date, more or less, with master and the dynamic allocation implementation?

cicecore/cicedynB/infrastructure/ice_grid.F90
real (kind=dbl_kind), parameter :: &
dxrect = 30.e5_dbl_kind ,&! uniform HTN (cm)
dyrect = 30.e5_dbl_kind ! uniform HTE (cm)
! real (kind=dbl_kind), public :: &
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be deleted.

@mattdturner
Copy link
Contributor Author

I assume it's up to date, more or less, with master and the dynamic allocation implementation?

I merged master into this branch on October 19. It includes the Fri Sep 28 commit of Alloc dyn4 (#194). So it should be up to date.

@apcraig
Copy link
Contributor

apcraig commented Oct 22, 2018

I ran a full test suite on conrad last night, hash 4626b06 https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks. Compared against the current master test suite run over the weekend. One thing, it looks like this branch is one PR behind master. Anyway, I think the results are probably OK. All the box results are different. I assume that's expected. The 1 degree test did not complete because it still has the incorrect namelist. This is fixed on the master with the latest PR. The alt01 and alt03 tests are different, although it's not clear exactly why. This is a little concerning. Is that expected. The results are very different. Looking at the log files, the first diagnostic is after 24 timesteps and it's very different. The ice area is different by a factor of 10 (less), the water balance has a bunch of terms zeroed out that weren't before. It looks like something important has changed. I don't know why it's just alt01 and alt03 that are affected. Is this something we need to understand.

In the diff below, it's (diff "new" "baseline")to < represents the new results.

> /p/home/apcraig/cice-consortium/cice.matt.box2001/testsuite.bx01/conrad_intel_restart_gx3_6x2_alt01.bx01>diff logs/cice.runlog.181022-001223 $WORKDIR/CICE_BASELINES_MASTER/cice.4671a1cb60.181021-000127/conrad_intel_restart_gx3_6x2_alt01/cice.runlog.181021-003247 
62d61
<   close_boundaries          =        F
68,70d66
<   coriolis                  = latitude
<   kridge                    =        1
<   ktransport                =        1
111c107
<   bgc_data_dir              = /uknown_bgc_data_dir
---
>   bgc_data_dir              = unknown_bgc_data_dir
322,325c318,321
< total ice area  (km^2) =    1.11487362338905167E+07   2.17446721276171021E+07
< total ice extent(km^2) =    1.11487362338912636E+07   2.17446721276185624E+07
< total ice volume (m^3) =    2.86493476407170586E+13   5.58781424233335000E+13
< total snw volume (m^3) =    2.14060204122054199E+12   4.17506420149698242E+12
---
> total ice area  (km^2) =    1.47407298426054806E+08   1.92905867664821625E+08
> total ice extent(km^2) =    1.56324900931266248E+08   2.04107714224489629E+08
> total ice volume (m^3) =    4.03222649989284062E+13   7.05731516956805625E+13
> total snw volume (m^3) =    2.14201601881120215E+12   4.17782204583553711E+12
328,329c324,325
< average albedo         =        0.81000000000000005       0.81000000000000028
< max ice volume     (m) =        2.56973947895774435       2.56973947895774435
---
> average albedo         =        0.70831954047609880       0.71239938402596259
> max ice volume     (m) =        2.57388329449594178       2.57388329449594178
335,342c331,338
< arwt evap h2o kg in dt =    0.00000000000000000E+00   0.00000000000000000E+00
< arwt frzl h2o kg in dt =    0.00000000000000000E+00   0.00000000000000000E+00
< arwt frsh h2o kg in dt =    0.00000000000000000E+00   0.00000000000000000E+00
< arwt ice mass (kg)     =    2.62714517865375440E+16   5.12402566021968160E+16
< arwt snw mass (kg)     =    7.06398673602778875E+14   1.37777118649400425E+15
< arwt tot mass (kg)     =    2.69778504601403240E+16   5.26180277886908240E+16
< arwt tot mass chng(kg) =    0.00000000000000000E+00   0.00000000000000000E+00
< arwt water flux        =    0.00000000000000000E+00   0.00000000000000000E+00
---
> arwt evap h2o kg in dt =   -3.14065194974866943E+12  -3.93173023171445703E+12
> arwt frzl h2o kg in dt =    5.27300119967046328E+13   6.62368055911166094E+13
> arwt frsh h2o kg in dt =   -2.64696243743919656E+14  -3.33722669005428875E+14
> arwt ice mass (kg)     =    3.69755170040173520E+16   6.47155801049390720E+16
> arwt snw mass (kg)     =    7.06865286207696750E+14   1.37868127512572725E+15
> arwt tot mass (kg)     =    3.76823822902250480E+16   6.60942613800648000E+16
> arwt tot mass chng(kg) =    3.14285603790864000E+14   3.96027744364800000E+14
> arwt water flux        =    3.14285603790875625E+14   3.96027744364831062E+14
344c340
< water flux error       =    0.00000000000000000E+00   0.00000000000000000E+00
---
> water flux error       =    3.08499603620219322E-16   4.69972723068647549E-16
346,352c342,348
< arwt atm heat flux (W) =    2.00677252210029300E+15   3.91404098297107500E+15
< arwt ocn heat flux (W) =    0.00000000000000000E+00   0.00000000000000000E+00
< arwt frzl heat flux(W) =    7.05454050741469920E+16   8.86156119504524640E+16
< arwt tot energy    (J) =   -9.01060205368686569E+21  -1.75744212814227298E+22
< arwt net heat      (J) =   -2.46739077187368092E+20  -3.04925655482932986E+20
< arwt tot energy chng(J)=    0.00000000000000000E+00   0.00000000000000000E+00
< arwt heat error        =    2.73831954532283359E-02   1.73505374999322788E-02
---
> arwt atm heat flux (W) =   -2.42665465720147080E+16  -3.05972704306722960E+16
> arwt ocn heat flux (W) =    2.57468797943184084E-21   5.02171230614227945E-21
> arwt frzl heat flux(W) =    4.89217333524982200E+15   6.14530362984248700E+15
> arwt tot energy    (J) =   -1.25859156849351658E+22  -2.20754833009416549E+22
> arwt net heat      (J) =   -1.04971391666152309E+20  -1.32273266617853215E+20
> arwt tot energy chng(J)=   -1.04971391666151752E+20  -1.32273266617942540E+20
> arwt heat error        =    4.42602679014267966E-17  -4.04636975699597543E-15
359,362c355,358
< arwt salt mass (kg)    =    1.05085807146150172E+14   2.04961026408787281E+14
< arwt salt mass chng(kg)=    0.00000000000000000E+00   0.00000000000000000E+00
< arwt salt flx in dt(kg)=    0.00000000000000000E+00   0.00000000000000000E+00
< arwt salt flx error    =    0.00000000000000000E+00   0.00000000000000000E+00
---
> arwt salt mass (kg)    =    1.47902068016069406E+14   2.58862320419756281E+14
> arwt salt mass chng(kg)=    1.04614516461391748E+12   1.31901317799086157E+12
> arwt salt flx in dt(kg)=   -1.04614516461395789E+12  -1.31901317799098901E+12
> arwt salt flx error    =   -2.73189374425175273E-16  -4.92313466260164250E-16
376,377c372,373
< avg ice thickness (m)  =        2.56973947895791621       2.56973947895791621
< avg snow depth (m)     =        0.19200400801603207       0.19200400801603207
---
> avg ice thickness (m)  =        2.57388329449611364       2.57388329449611364
> avg snow depth (m)     =        0.19213083652475232       0.19213083652475232
380c376
< surface temperature(C) =      -20.14999999999998082     -20.14999999999998082
---
> surface temperature(C) =      -24.57951004561716246     -24.57951004561716246
382,385c378,381
< outward longwave flx   =        0.00000000000000000       0.00000000000000000
< sensible heat flx      =        0.00000000000000000       0.00000000000000000
< latent heat flx        =        0.00000000000000000       0.00000000000000000
< subl/cond (m ice)      =        0.00000000000000000       0.00000000000000000
---
> outward longwave flx   =     -214.66891827616245791    -214.66891827616245791
> sensible heat flx      =       18.62840040205053782      18.62840040205053782
> latent heat flx        =        1.36331636320478089       1.36331636320478089
> subl/cond (m ice)      =        0.00000188789027464       0.00000188789027464
389,390c385,386
< new ice (m)            =        0.00000000000000000       0.00000000000000000
< congelation (m)        =        0.00000000000000000       0.00000000000000000
---
> new ice (m)            =        0.00000000000000038       0.00000000000000038
> congelation (m)        =        0.00017251622852200       0.00017251622852200
393,395c389,391
< effective dhi (m)      =        0.00000000000000000       0.00000000000000000
< effective dhs (m)      =        0.00000000000000000       0.00000000000000000
< intnl enrgy chng(W/m^2)=        0.00000000000000000       0.00000000000000000
---
> effective dhi (m)      =        0.00017251622852221       0.00017251622852221
> effective dhs (m)      =        0.00000524604661165       0.00000524604661165
> intnl enrgy chng(W/m^2)=       14.83781797137525338      14.83781797137525338
400c396
< heat used (W/m^2)      =        0.00000000000000000       0.00000000000000000
---
> heat used (W/m^2)      =       -0.00000000000000000      -0.00000000000000000

@rallard77
Copy link
Contributor

rallard77 commented Oct 22, 2018 via email

@rallard77
Copy link
Contributor

rallard77 commented Oct 22, 2018 via email

@apcraig
Copy link
Contributor

apcraig commented Oct 22, 2018

You can see the timing info on the results page. It looks fine as far as I can see. The timing column has 3 timers each with current time and then baseline time in parenthesis. These are short runs and there is always some variability, but there is no consistent slowdown from what I can see.

@eclare108213
Copy link
Contributor

I don't expect the results to be different. Maybe for the box tests, but not other configurations. The results are so different, my first suspicion is that the forcing has changed. We need to figure this out before merging.

Update CICE_RunMod conditionals from 'ktherm > 0' to 'ktherm >= 0' si…
5e1b713 
…nce a ktherm value of 0 does NOT mean thermodynamics is turned off
@mattdturner
Copy link
Contributor Author

Rick noticed that the issues appeared to be related to the ktherm namelist variable. I noticed in CICE_RunMod.F90 that we used if (ktherm > 0) then to disable thermodynamics. However, a ktherm value of 0 does not mean thermodynamics is disabled. I updated the conditionals to read if (ktherm >= 0) then so that thermodynamics is only disabled if ktherm = -1. I believe that this should resolve the problem, but it should be tested.

I'm also not sure what the CICE_RunMod.F90_debug file is, or if it should be in the repository or not. But I updated that file as well.

@apcraig
Copy link
Contributor

apcraig commented Oct 22, 2018

I'm running a full suite on conrad with intel to see if it's fixed.

@eclare108213
Copy link
Contributor

That makes a lot of sense. The tests that failed are likely the ones that have ktherm=0 set.

CICE_RunMod.F90_debug is supposed to be identical to CICE_RunMod.F90, but with the addition of calls to a debugging routine that prints tons of information. There's probably a better way to do this without wrapping all of those calls in "if debug" conditionals. I've kept it as a separate file because it's rarely used -- but maybe it's rarely used because it's a separate file and no one else knows what it's for...

@apcraig
Copy link
Contributor

apcraig commented Oct 22, 2018

I think we're OK now based on the conrad intel results. I think we can merge. thanks.

@apcraig
Copy link
Contributor

apcraig commented Oct 22, 2018

Does anyone think we should wait to merge?

@eclare108213
Copy link
Contributor

no need to wait -- go for it

@apcraig apcraig merged commit 83686a3 into CICE-Consortium:master Oct 22, 2018
@apcraig
Copy link
Contributor

apcraig commented Oct 22, 2018

Done. And just for fun, I'm going to run a full test suite on conrad again with master.

@phil-blain phil-blain mentioned this pull request Nov 16, 2018
Closed
@mattdturner mattdturner deleted the box2001 branch June 24, 2019 14:52
@eclare108213 eclare108213 mentioned this pull request Dec 2, 2020
Merged
16 tasks
phil-blain added a commit to phil-blain/CICE that referenced this pull request Jun 2, 2021
@phil-blain
drivers/hadgem3/CICE_RunMod: add call to 'save_init'
525b21e 
The hadgem3 driver was not updated when 'save_init' was added in 83686a3
(Implement box model test from 2001 JCP paper (CICE-Consortium#151), 2018-10-22). As
this subroutine is necessary to ensure proper initialization of the
model, add it now.
apcraig pushed a commit that referenced this pull request Jun 10, 2021
@phil-blain
Add ECCC driver for coupling with NEMO, update 'hadgem3' driver (#605)
a63cc1c 
* drivers/hadgem3: add missing 'subname' and use existing 'subname's

* drivers/hadgem3/CICE_InitMod: update 'init_lvl' call

Add the required 'iblk' argument.

* drivers/hadgem3/CICE_RunMod: remove uneeded 'dt' arguments

The subroutines 'prep_radiation', 'zsal_diags', 'bgc_diags' and 'hbrine_diags'
do not take a 'dt' argument anymore, so remove it.

* drivers/hadgem3/CICE_RunMod: get 'Lsub' from Icepack

* drivers/hadgem3/CICE_RunMod: remove 'da_state_update' subroutine

This subroutine is inside an 'ICE_DA' CPP, which is not used in
any configuration. Remove it.

* drivers/hadgem3/CICE_RunMod: remove stray '+'

This '+' sign was copy-pasted there in error in 29b99b6 (CICE: Floe size
distribution  (#382), 2019-12-07). Remove it.

* drivers/hadgem3: remove obsolete 'check_finished_file' subroutine

Remove the call to 'check_finished_file' as well as the definition
of the subroutine, as the 'hadgem3' driver is not used on machine 'bering'
and it's unclear if machine 'bering' still exists.

* drivers/hadgem3: fix cice_init so it calls 'count_tracers'

This was forgotten back in 8b0ae03 (Refactor tracer initialization (#235), 2018-11-16)

* drivers/hadgem3/CICE_RunMod: add call to 'save_init'

The hadgem3 driver was not updated when 'save_init' was added in 83686a3
(Implement box model test from 2001 JCP paper (#151), 2018-10-22). As
this subroutine is necessary to ensure proper initialization of the
model, add it now.

* drivers/hadgem3/CICE_RunMod: tweak loop indices in 'coupling_prep'

Other drivers use 'ilo,ihi' and 'jlo,jhi' here. Do the same.

* drivers/hagdem3: update driver to new time manager

* drivers/hadgem3: pass 'mpi_comm_opa' explicitely to init_communicate

In 066070e (Fix minor issues in documentation, key_ CPPs, bfbcomp return
codes (#532), 2020-11-23), the 'ice_communicate' module was updated to
remove CPP macros relating to the OASIS coupler (key_oasis*) and to the
NEMO ocean model (key_iomput). These CPPs were used to make the correct
MPI communicator accessible to the 'init_communicate' subroutine.
However, that subroutine already accepts an optional MPI communicator as
argument and it was deemed cleaner to require the driver layer to
explicitely pass the communicator instead of making it accessible
through 'use' statements.

Update the 'hadgem3' driver, used for coupling with NEMO, to explicitely
pass the NEMO MPI communicator 'mpi_comm_opa' to 'init_communicate'.

* drivers: add 'nemo_concepts' driver

Historically the 'hadgem3' driver has been used when compiling a single
NEMO-CICE executable at ECCC.

Going forward, all driver-level adjustements will be done in our own
driver, 'nemo_concepts', 'CONCEPTS' being the name of the
multi-departmental collaboration around using the NEMO ocean model.

Copy CICE_InitMod, CICE_RunMod and CICE_FinalMod from the 'hadgem3'
directory to a new 'nemo_concepts' directory under 'drivers/direct'.

The following commits will clean up this new driver and port over some
in-house adjustments.

* drivers/nemo_concepts: remove unused 'writeout_finished_file' subroutine

This subroutine was only called on machine 'bering', which is not an
ECCC machine and probably does not exist anymore anyway. Remove it.

* drivers/nemo_concepts: call 'scale_fluxes' with 'aice_init'

Since 'merge_fluxes' is called with aice_init, it is more consistent to
also call 'scale_fluxes', in 'coupling_prep' with 'aice_init'
instead of 'aice'.

Copy this in-house change to the new 'nemo_concepts' driver.
phil-blain added a commit to phil-blain/CICE that referenced this pull request Apr 8, 2022
@phil-blain
drivers/nemo_concepts: let 'ktherm=-1' only disable thermodynamics
53587b1 
When CICE learned to disable the thermodynamics in 83686a3 (Implement
box model test from 2001 JCP paper (CICE-Consortium#151), 2018-10-22), it was not
envisioned that this capability would be used with 'calc_strair =
.true.'. This currently does not work (the atmospheric stresses are
always zero). This is because they are computed in
'icepack_atmo::icepack_atm_boundary', called from
'icepack_therm_vertical::icepack_step_therm1', itself called from
'ice_step_mod::step_therm1', a call which is skipped if thermodynamics
is disabled ('ktherm<0').

Adjust the logic in ice_step to always call 'step_therm1', and update
Icepack to a commit in which some parts of the computation in
'icepack_step_therm1' are only done if 'ktherm >= 0', while others
(namely the computation of the atmospheric stresses) are always done.
phil-blain added a commit to phil-blain/CICE that referenced this pull request Apr 12, 2022
@phil-blain
drivers/nemo_concepts: let 'ktherm=-1' only disable thermodynamics
78ffa02 
When CICE learned to disable the thermodynamics in 83686a3 (Implement
box model test from 2001 JCP paper (CICE-Consortium#151), 2018-10-22), it was not
envisioned that this capability would be used with 'calc_strair =
.true.'. This currently does not work (the atmospheric stresses are
always zero). This is because they are computed in
'icepack_atmo::icepack_atm_boundary', called from
'icepack_therm_vertical::icepack_step_therm1', itself called from
'ice_step_mod::step_therm1', a call which is skipped if thermodynamics
is disabled ('ktherm<0').

Adjust the logic in ice_step to always call 'step_therm1', and update
Icepack to a commit in which some parts of the computation in
'icepack_step_therm1' are only done if 'ktherm >= 0', while others
(namely the computation of the atmospheric stresses) are always done.
phil-blain added a commit to phil-blain/CICE that referenced this pull request Jan 26, 2024
@phil-blain
drivers/nemo_concepts: let 'ktherm=-1' only disable thermodynamics
7d61199 
When CICE learned to disable the thermodynamics in 83686a3 (Implement
box model test from 2001 JCP paper (CICE-Consortium#151), 2018-10-22), it was not
envisioned that this capability would be used with 'calc_strair =
.true.'. This currently does not work (the atmospheric stresses are
always zero). This is because they are computed in
'icepack_atmo::icepack_atm_boundary', called from
'icepack_therm_vertical::icepack_step_therm1', itself called from
'ice_step_mod::step_therm1', a call which is skipped if thermodynamics
is disabled ('ktherm<0').

Adjust the logic in ice_step to always call 'step_therm1', and update
Icepack to a commit in which some parts of the computation in
'icepack_step_therm1' are only done if 'ktherm >= 0', while others
(namely the computation of the atmospheric stresses) are always done.
phil-blain added a commit to phil-blain/CICE that referenced this pull request Feb 9, 2024
@phil-blain
drivers/nemo_concepts: let 'ktherm=-1' only disable thermodynamics
d023026 
When CICE learned to disable the thermodynamics in 83686a3 (Implement
box model test from 2001 JCP paper (CICE-Consortium#151), 2018-10-22), it was not
envisioned that this capability would be used with 'calc_strair =
.true.'. This currently does not work (the atmospheric stresses are
always zero). This is because they are computed in
'icepack_atmo::icepack_atm_boundary', called from
'icepack_therm_vertical::icepack_step_therm1', itself called from
'ice_step_mod::step_therm1', a call which is skipped if thermodynamics
is disabled ('ktherm<0').

Adjust the logic in ice_step to always call 'step_therm1', and update
Icepack to a commit in which some parts of the computation in
'icepack_step_therm1' are only done if 'ktherm >= 0', while others
(namely the computation of the atmospheric stresses) are always done.
phil-blain added a commit to phil-blain/CICE that referenced this pull request Feb 12, 2024
@phil-blain
drivers/nemo_concepts: let 'ktherm=-1' only disable thermodynamics
4621242 
When CICE learned to disable the thermodynamics in 83686a3 (Implement
box model test from 2001 JCP paper (CICE-Consortium#151), 2018-10-22), it was not
envisioned that this capability would be used with 'calc_strair =
.true.'. This currently does not work (the atmospheric stresses are
always zero). This is because they are computed in
'icepack_atmo::icepack_atm_boundary', called from
'icepack_therm_vertical::icepack_step_therm1', itself called from
'ice_step_mod::step_therm1', a call which is skipped if thermodynamics
is disabled ('ktherm<0').

Adjust the logic in ice_step to always call 'step_therm1', and update
Icepack to a commit in which some parts of the computation in
'icepack_step_therm1' are only done if 'ktherm >= 0', while others
(namely the computation of the atmospheric stresses) are always done.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eclare108213 eclare108213 eclare108213 approved these changes

@apcraig apcraig apcraig approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@mattdturner @rallard77 @apcraig @eclare108213 @duvivier