Mosaic forcing clean #500

davidclemenssewall · 2024-09-24T20:24:48Z

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

PR checklist

Observational data presents the following complications:

Missing values. Real observational datasets almost always have missing values (power loss, radiometer dome iced over, etc...). Furthermore, future versions of the observational datasets may fill some of these missing values (e.g., by reprocessing data that previously failed qc).
Variable frequencies. Different data streams may be measured at different sampling frequencies. Data might be measured more often than the model's timestep (thus requiring aggregation) or less often (thus requiring interpolation).
Different start and end time points. Different data streams may have different start or end times, and updated versions of datasets may extend the time domain.
Merged observational datasets are a work-in-progress. The 3 challenges above all touch on the fact that for sea ice field campaigns, it can take years from the end of the campaign until a truly finalized observational dataset is available to force Icepack with (e.g., MOSAiC is 4 years and counting, SHEBA still does not have such a dataset). We still want to be able to use campaign data to support Icepack development during this time, without having to change the Icepack driver every time the forcing dataset is updated.

Additionally, for comparing with field data it is very useful to be able to easily adjust:

The initial state of the sea ice and ocean mixed layer.
Fixed values for the oceanic heat flux convergence (qdp) and sea surface salinity (sss).

To address these challenges the PR provides the following features:

Precalculate the forcing values at each timestep on initialization (in init_forcing). The current paradigm for forcing data in Icepack is that when init_forcing is called, a subroutine for the specific dataset (e.g., atm_CFS) stores the forcing data in the *_data in essentially the same format that the raw data is present in, without timestamp information. It also hardcodes the maximum size that a forcing dataset can be to 8760. Then, at each timestep the get_forcing subroutine has a code block for each forcing dataset that contains the timestamp information and interpolates the forcing data to the given timestep. Under this paradigm, I could not figure out a graceful way to handle missing values, nor forcing data with higher sampling frequency than the Icepack timestep, nor changing start times of the forcing dataset. This PR adds a new option, precalc_forc, for use with the MOSAiC forcing. When precalc_forc = .true., the forcing data is aggregated/interpolated to the Icepack timestep in the call to init_forcing and stored the the *_data arrays. Then, the get_forcing subroutine merely indexes into the *_data arrays.
Read forcing data from MDF-formatted netCDF files (Uttal et al. 2024; https://doi.org/10.5194/gmd-17-5225-2024, https://gitlab.com/mdf-makers/mdf-toolkit). I chose this format because its framework seemed well-aligned, but it is still in development so we can add variables to it that we need for the ice model (e.g., mixed layer depth).
To handle variable frequencies and missing data, forcing data are first temporally-averaged for each timestep and then interpolated. For a give data variable (var_data), The MOSAiC_average subroutine takes the average of all forcing datapoints within each timestep +- 0.5 dt excluding missing values and stores the results in var_data. If there is no valid data within 0.5 dt of a given timestep (e.g., most timesteps if the model timestep is much smaller than the sampling period) then a missing value is placed in var_data(timestep). Then, the MOSAiC_interpolate subroutine linearly interpolates missing values in var_data.
The option to explicitly set in the namelist the initial ice and snow thicknesses for the slab and ITD grid cells and the SST for the 3 ice grid cells.
The option to explicitly set in the namelist fixed ocean mixed layer conditions qdp_fixed, sss_fixed, and hmix_fixed. These are overwritten by forcing data.

Outstanding issues:

~~Automatically detect cadence of the inputs and adjust accordingly (currently the code requires that the atmospheric forcing is at 1 minute cadence and that the oceanic forcing is at daily cadence)~~
~~It is not obvious to a user that setting the atmospheric forcing (atm_data_type = 'clim') can overwrite the oceanic heat flux convergence.~~
~~The atm_MOSAiC and ocn_MOSAiC subroutines are nearly identical except for the data variables. It would probably be cleaner to combine them into a single subroutine.~~
~~Perhaps input checking should be added to prevent users from setting precalc_forc = .true. with non MOSAiC forcings.~~
Update documentation to include:
- How driver handles MDF formatted forcing
- MOSAiC forcing
- Initial ice and SST conditions in namelist
- Fixed ocean mixed layer conditions in namelist
~~Add warning in log for if MOSAiC time domain includes leg 4-5 transition (observatory relocation)~~
Add MOSAiC test case
Add link to MOSAiC forcing data to wiki

…gfault

Just updating for_igs to consortium main. Only conflict was in icedrv_init use section

…onsortium main

…names

…in filenames

davidclemenssewall · 2024-09-24T21:14:24Z

@apcraig When I try running the base_suite on Windows Subsystem for Linux (-m conda -e linux), I fail the run stage for the conda_linux_smoke_col_1x1_alt03_debug_run1year and conda_linux_smoke_col_1x1_alt04_debug_run1year tests due to a floating-point exception. I've copied the relevant sections from the runlog below. Strangely, this error does not arise in the GH macos-testing. It also does not occur when I run these tests with the main branch locally on WSL. So it seems to be some combination of the platform and the code changes, although I cannot see how these code changes in the driver cause this error in the atmospheric stability. I was hoping you might have a suggestion on how I can fix this. Thanks!

Tail from the icepack runlog:

  Initial forcing data year =         2015
  Final   forcing data year =         2015
 Reading /root/icepack-dirs/input/Icepack_data/forcing/SHEBA/open_clos_lindsay.dat                                      


Program received signal SIGFPE: Floating-point exception - erroneous arithmetic operation.

Backtrace for this error:
#0  0x7f05dd5e951f in ???
#1  0x557a0eb656c2 in compute_stability_parameter
        at /root/code/CICE/Icepack/columnphysics/icepack_atmo.F90:992
#2  0x557a0eb67de2 in atmo_boundary_layer
        at /root/code/CICE/Icepack/columnphysics/icepack_atmo.F90:276
#3  0x557a0eb65d98 in __icepack_atmo_MOD_icepack_atm_boundary
        at /root/code/CICE/Icepack/columnphysics/icepack_atmo.F90:954
#4  0x557a0eaf4c59 in __icedrv_step_MOD_ocean_mixed_layer
        at /root/code/CICE/Icepack/configuration/driver/icedrv_step.F90:1251
#5  0x557a0ea69c25 in coupling_prep
        at /root/code/CICE/Icepack/configuration/driver/icedrv_RunMod.F90:303
#6  0x557a0ea6d57b in __icedrv_runmod_MOD_ice_step
        at /root/code/CICE/Icepack/configuration/driver/icedrv_RunMod.F90:221
#7  0x557a0ea6d6dc in __icedrv_runmod_MOD_icedrv_run
        at /root/code/CICE/Icepack/configuration/driver/icedrv_RunMod.F90:57
#8  0x557a0ea6944d in icedrv
        at /root/code/CICE/Icepack/configuration/driver/icedrv_MAIN.F90:51
#9  0x557a0ea69753 in main
        at /root/code/CICE/Icepack/configuration/driver/icedrv_MAIN.F90:28

apcraig · 2024-09-24T23:26:22Z

I don't know anything about Windows systems, but I just tested your branch on derecho for the "alt04_debug_run1year". It passed with intel, fails with gnu with exactly the same traceback as above. So this is a problem in the code modifications. If you need me to help you debug, let me know.

apcraig · 2024-09-24T23:39:42Z

Doing just some quick debugging, the value of hol at line 992 of icepack_atm.F90 is a NaN. Going up one line, tstar and qstar are both NaN in the computation of hol. There are several forcing options in Icepack, is it possible that your changes work for mosaic and a subset of the forcing, but not all forcing settings?

… of sst

davidclemenssewall · 2024-09-25T22:11:15Z

Thanks for the help! With that info I was able to track down the source of the 'run' error to be due to the initialization of the sstdat array in icedrv_flux.F90 from the sst array. One of the changes that I'm proposing with this PR is to allow the user to explicitly set the initial SST, which I think can be useful when looking initializing before freeze-up or during the melt season when the mixed layer is relatively fresh. But I had failed to propagate that change to the sstdat array.

The latest commit now passes all of the 'run' tests on WSL with the GNU compiler. But it is failing the baseline comparison with the main branch for all of the tests that use the alt0[1-4] namelist options (see failing tests below). It appears to me the shared factor in the tests that failed is that they contain ktherm = 1 conduct = 'bubbly'. Whereas the other tests either have ktherm = 2 or, if they use Bitz and Lipscomb thermodynamics they use MU71 conductivity.

Any suggestions would be welcome, I'll keep working on this.

Failing test results:

(base) root@dsewall-wl-psl:~/code/CICE/Icepack/testsuite.mosaic_forcing_clean# ./results.csh | grep FAIL
FAIL conda_linux_smoke_col_1x1_alt01_debug_run1year compare icepack.main different-data
FAIL conda_linux_smoke_col_1x1_alt02_debug_run1year compare icepack.main different-data
FAIL conda_linux_smoke_col_1x1_alt03_debug_run1year compare icepack.main different-data
FAIL conda_linux_smoke_col_1x1_alt04_debug_run1year compare icepack.main different-data
FAIL conda_linux_restart_col_1x1_alt01 compare icepack.main different-data
FAIL conda_linux_restart_col_1x1_alt02 compare icepack.main different-data
FAIL conda_linux_restart_col_1x1_alt03 compare icepack.main different-data
FAIL conda_linux_restart_col_1x1_alt04 compare icepack.main different-data
 FAIL - test failed
8 of 215 tests FAILED

…d qdp_data = -6.0 back to atm_climatological, note that now the atm_data_type can overwrite the ocean forcing

davidclemenssewall · 2024-09-27T16:23:16Z

The most recent commit fixes this issue and all checks now pass with identical output.

apcraig · 2024-09-27T19:34:10Z

Should we add a couple new tests to the test suite for mosaic? I assume this would require the mosaic forcing datasets be shared on zenodo? What's the plan for sharing that forcing dataset as well as any mosaic data "for comparisons" with respect to the Consortium?

@davidclemenssewall, there is a bunch of trailing whitespace in your code changes which we try to avoid. You can clean it up quickly by doing

sed -i 's/[ \t]*$//' $file

where $file is the file to be cleaned. I will also check and fix this before the next release. thanks,

tony.......

davidclemenssewall · 2024-09-27T20:22:56Z

Fixed the trailing whitespace, thanks for the shell command!

I agree that we should have some tests with this. I think at minimum there should be a run with the 'atmmosaic' and 'ocnmosaic' initialized in November 2019 with namelist options to set the initial snow and ice thicknesses and run through July of 2020. I'm not sure if it makes sense to include some tests of different physics options with this forcing as well? I don't think any of the other forcing options provide daily updates to qdp, sss, and hmix so perhaps having the new congelation growth option would be nice to test.

The forcing datasets are currently archived at ADC: https://arcticdata.io/catalog/view/doi%3A10.18739%2FA2GX44W6J. We expect to be updating the MOSAiC forcing and evaluation (or comparison) datasets for the next couple years at least. So I'm not sure if it makes sense to add it to the existing tarball, or add it as another row in the table on this page: https://github.com/CICE-Consortium/Icepack/wiki/Icepack-Input-Data. It is also a little bit different from the existing forcing datasets in that users would be encouraged to use it for scientific results as well as code testing. Perhaps we should have a meeting to discuss?

apcraig · 2024-09-27T20:33:03Z

Thanks @davidclemenssewall.

If we are going to have some mosaic tests in the test suite, then we'll need to have a reasonably stable forcing dataset that we can install on all the test machines. We'll want to put it on zenodo and have it be part of the Icepack inputdata. Happy to have a broader discussion about how to best proceed.

@davidclemenssewall, if you could propose one (or a few) test cases that we could add to the test suite, that would be great. You can use multiple "options" including 'atmmosaic' and 'ocnmosaic' as needed and turn on the appropriate physics. You can create some set_nml if needed and add them to the base_suite.ts if you'd like. For our testing, we only really need enough test cases to cover the code productively. We do not need to cover all the various physics combinations that a user might find reasonable. We need to be somewhat clever to cover "everything" while still limiting the number of tests as much as possible.

eclare108213 · 2024-09-29T12:57:55Z

Outstanding issues:

Automatically detect cadence of the inputs and adjust accordingly (currently the code requires that the atmospheric forcing is at 1 minute cadence and that the oceanic forcing is at daily cadence)

If this is easy to implement, let's do it. Otherwise I'd rather merge the PR earlier and improve it later.

It is not obvious to a user that setting the atmospheric forcing (atm_data_type = 'clim') can overwrite the oceanic heat flux convergence.

This could be noted in the documentation and also as a warning in the diagnostics log file when namelist values are written out.

The atm_MOSAiC and ocn_MOSAiC subroutines are nearly identical except for the data variables. It would probably be cleaner to combine them into a single subroutine.

Your choice.

Perhaps input checking should be added to prevent users from setting precalc_forc = .true. with non MOSAiC forcings.

Definitely.

I also recommend adding the forcing to our datasets and creating a few options for testing, including one that represents a 'production' configuration. I will still recommend that our codes be used in coupled configurations to capture important feedbacks, but we can adjust our stern warnings in the Icepack docs to acknowledge the MOSAiC configuration as a tool to be used thoughtfully.

…orcing

dabail10 · 2024-10-09T14:19:29Z

HI all. Let's discuss this at the meeting tomorrow. I can understand that it would be nice to have this as a part of the Icepack forcing dataset on zenodo. However, the National Science Foundation requires the datasets be put somewhere like the Arctic Data Center where they do reside currently. This would mean duplicating the data on zenodo. Is there a way to have a "link" from zenodo to the ADC?

anton-seaice

Thanks for doing this David! This will be super useful

As Elizabeth noted, it needs some documentation.

And when adding the link to the forcing data to the wiki, I think the DOI link is best:

e.g. https://doi.org/10.18739/A2GX44W6J

anton-seaice · 2024-10-10T20:34:20Z

configuration/driver/icedrv_forcing.F90

      character(char_len), public :: &
         atm_data_format, & ! 'bin'=binary or 'nc'=netcdf
         ocn_data_format, & ! 'bin'=binary or 'nc'=netcdf
         bgc_data_format, & ! 'bin'=binary or 'nc'=netcdf
-         atm_data_type,   & ! 'default', 'clim', 'CFS'
-         ocn_data_type,   & ! 'default', 'SHEBA'
+         atm_data_type,   & ! 'default', 'clim', 'CFS', 'MOSAiC'


I think we could use a more general name than MOSAIC for the new data type - its supports any 'MDF' file right ?

Thanks for the suggestions Anton! @eclare108213 and @apcraig, what would you think about making the atm_data_type and ocn_data_type for this 'MDF' instead of 'MOSAiC'? Then I would also propose that we make the default path for these forcing data: /icepack-dirs/input/Icepack_data/forcing/MDF.

anton-seaice · 2024-10-10T20:35:59Z

configuration/driver/icedrv_flux.F90

+      ! fixed ocean mixed layer properties (are overwritten by forcing data)
+      real (kind=dbl_kind), public :: &
+         sss_fixed   , & ! Sea surface salinity (PSU)
+         qdp_fixed   , & ! Deep ocean heat flux (negative upward, W/m^2)
+         hmix_fixed      ! Mixed layer depth (m)
+


My preference would be to do this in a different PR, so its clear what the scope of the change is (i.e. its not related to MOSAIC forcing data)

I totally agree that this would be best practice. But I admit that I'm loath to do so because the branch I originally made these modifications on is a bit stale and it doesn't feel like a productive use of time to sort through the merge conflicts and do all of the testing if the ultimate outcome for the code is the same. If others feel strongly about it I will try to get to it sometime next week. @apcraig @eclare108213 @dabail10 What do you think?

anton-seaice · 2024-10-10T20:42:01Z

configuration/driver/icedrv_forcing.F90

+         call icedrv_system_abort(string=subname//&
+         ' ERROR: only NetCDF input implemented for atm_MOSAiC', &
+         file=__FILE__,line=__LINE__)


I think it's clearer if you put this at line 1087, if:

if (atm_data_format != 'nc') then ... abort

Done. This will be fixed in the next push.

…s not run

davidclemenssewall · 2024-10-11T17:13:49Z

Outstanding issues:

Automatically detect cadence of the inputs and adjust accordingly (currently the code requires that the atmospheric forcing is at 1 minute cadence and that the oceanic forcing is at daily cadence)

If this is easy to implement, let's do it. Otherwise I'd rather merge the PR earlier and improve it later.

It is not obvious to a user that setting the atmospheric forcing (atm_data_type = 'clim') can overwrite the oceanic heat flux convergence.

This could be noted in the documentation and also as a warning in the diagnostics log file when namelist values are written out.

The atm_MOSAiC and ocn_MOSAiC subroutines are nearly identical except for the data variables. It would probably be cleaner to combine them into a single subroutine.

Your choice.

Perhaps input checking should be added to prevent users from setting precalc_forc = .true. with non MOSAiC forcings.

Definitely.

I also recommend adding the forcing to our datasets and creating a few options for testing, including one that represents a 'production' configuration. I will still recommend that our codes be used in coupled configurations to capture important feedbacks, but we can adjust our stern warnings in the Icepack docs to acknowledge the MOSAiC configuration as a tool to be used thoughtfully.

Latest commit addresses these with the exception of the ~~warnings~~, documentation updates, and MOSAiC testing. Working on those currently...

…nic heat flux convergence

davidclemenssewall and others added 28 commits May 8, 2023 13:55

up to reading time01

f61b4b1

ran into ntime /= npt issue

8d0eb98

made *_data arrays allocatable to solve ntime issue, now getting a se…

92b7cf3

…gfault

forgot to allocate open_data and clos_data, fixed and solved seg fault

52af4e1

averaging function runs

4c9fed7

Now working for Tair

8c99a08

works okay, qa/1000 and fsnow and frain set to 0

e9cac2d

dummy ocean forcing and initial conditions closer to mosaic

3409f42

fixed istep /= istep1 issue, now can start at times other than Jan. 1

7055333

Merge branch 'main' into for_igs

312b24c

Just updating for_igs to consortium main. Only conflict was in icedrv_init use section

updated forcing code now that MDF doesn't write calendar

c97bc56

added functionality for precipitation and ocean mixed layer forcing

c69f403

partially through changes to add ice thickness init options

71a176c

init ice thickness complete and checked that results match

78f761c

set initial sst working

9db2ef6

removed setting sst from icedrv_flux

9f297c1

fixed sss, qdp, and hmix all working

f00f46b

Merge branch 'main' into mosaic_ocn_forcing to stay up to date with c…

5a6ae48

…onsortium main

Merge branch 'init_options' into mosaic_ocn_forcing

6219577

change atmmosaic options

397e09a

updated atmmosaic and ocnmosaic to new forcings

dd92954

updated names for downwelling radiation fields in forcing to correct …

2c8195a

…names

updated mosaic settings to account for fact that ADC converts - to _ …

e6c0dce

…in filenames

Merge branch 'main' into mosaic_forcing_clean

6be69b0

Merge branch 'main' into mosaic_forcing_clean

ac5ad88

Cleaned up extraneous comments and renamed precalc_forc

c3a815d

added check for standard calendar and leap years

946fbcb

Merge branch 'main' into mosaic_forcing_clean

38995e7

davidclemenssewall mentioned this pull request Sep 24, 2024

MOSAiC forcing from Xiaochun Wang #488

Open

davidclemenssewall added 2 commits September 25, 2024 11:24

changed driver such that sstdat is initialized from sst_init, instead…

0eb1066

… of sst

reset sst initialization to align with main

3c00c2f

set qdp_fixed to 0.0 to align with ocn_data_type = 'default' and adde…

ab1fd79

…d qdp_data = -6.0 back to atm_climatological, note that now the atm_data_type can overwrite the ocean forcing

fixed trailing whitespace

12a692b

eclare108213 requested review from eclare108213, apcraig and dabail10 September 29, 2024 12:38

eclare108213 added the Forcing label Sep 29, 2024

davidclemenssewall added 2 commits September 30, 2024 12:54

Merge branch 'main' into mosaic_forcing_clean

0b4b404

added check to ensure that precalc_forc is only used for the MOSAiC f…

b47d4c8

…orcing

anton-seaice reviewed Oct 10, 2024

View reviewed changes

davidclemenssewall added 3 commits October 10, 2024 16:34

forcing refactoring and flexible cadence code complete, compiles, doe…

7d88cbf

…s not run

fixed typo, now runs successfuly

d93b3fc

reorganized data format checks for clarity

b15d4d5

davidclemenssewall added 2 commits October 11, 2024 15:35

added warning for MOSAiC leg 4-5 transition

429e824

added warning for impact of climatological atmosphere forcing on ocea…

358896d

…nic heat flux convergence

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mosaic forcing clean #500

Mosaic forcing clean #500

davidclemenssewall commented Sep 24, 2024 •

edited

Loading

davidclemenssewall commented Sep 24, 2024

apcraig commented Sep 24, 2024

apcraig commented Sep 24, 2024

davidclemenssewall commented Sep 25, 2024

davidclemenssewall commented Sep 27, 2024

apcraig commented Sep 27, 2024

davidclemenssewall commented Sep 27, 2024

apcraig commented Sep 27, 2024

eclare108213 commented Sep 29, 2024

dabail10 commented Oct 9, 2024

anton-seaice left a comment

anton-seaice Oct 10, 2024

davidclemenssewall Oct 11, 2024

anton-seaice Oct 10, 2024

davidclemenssewall Oct 11, 2024

anton-seaice Oct 10, 2024

davidclemenssewall Oct 11, 2024

davidclemenssewall commented Oct 11, 2024 •

edited

Loading

Mosaic forcing clean #500

Are you sure you want to change the base?

Mosaic forcing clean #500

Conversation

davidclemenssewall commented Sep 24, 2024 • edited Loading

PR checklist

davidclemenssewall commented Sep 24, 2024

apcraig commented Sep 24, 2024

apcraig commented Sep 24, 2024

davidclemenssewall commented Sep 25, 2024

davidclemenssewall commented Sep 27, 2024

apcraig commented Sep 27, 2024

davidclemenssewall commented Sep 27, 2024

apcraig commented Sep 27, 2024

eclare108213 commented Sep 29, 2024

dabail10 commented Oct 9, 2024

anton-seaice left a comment

Choose a reason for hiding this comment

anton-seaice Oct 10, 2024

Choose a reason for hiding this comment

davidclemenssewall Oct 11, 2024

Choose a reason for hiding this comment

anton-seaice Oct 10, 2024

Choose a reason for hiding this comment

davidclemenssewall Oct 11, 2024

Choose a reason for hiding this comment

anton-seaice Oct 10, 2024

Choose a reason for hiding this comment

davidclemenssewall Oct 11, 2024

Choose a reason for hiding this comment

davidclemenssewall commented Oct 11, 2024 • edited Loading

davidclemenssewall commented Sep 24, 2024 •

edited

Loading

davidclemenssewall commented Oct 11, 2024 •

edited

Loading