escomp/cesm_share

CESM shared code

cesm_share's Issues

shr_pio_mod needs refactor

This module is too mct oriented and needs to be rewritten for cmeps/cdeps

shr_flux_mod update to use shr_wv_sat_mod

Moving from ESMCI/cime#854

From @jedwards4b original comment in that issue (from 2016):

This issue is moved from CESM-DEVELOPMENT issue 39.
shr_flux_mod should use the saturation vapor pressure from shr_wv_sat_mod instead of
having it's own method.
Any of the common approximation methods will give effectively the same answer at reasonable temperatures/pressures over the ocean, and this is not performance-critical code that needs optimization. Still, it's better practice to call the standard routine than to have a series of duplicated/hard-coded formulas, so this should probably be cleaned up some time.

The one caveat is that there's a factor of 0.98 applied to the humidity, which roughly adjusts for the ocean salinity. That factor needs to be kept, in the subroutines where it is present to begin with.

Run Fortran (pfunit-based) unit tests on each PR

As part of the testing workflow, we should run the Fortran (pfunit-based) unit tests on each PR. Ideally, most or all of this repository would be covered by unit tests, and the unit testing together with the existing scripts_regression_tests would be sufficient coverage for a CESM_share PR to be merged.

See #17 (review) for the current process, though #18 suggests how we could make this more straightforward – but I think from an automated testing perspective it might not make much difference.

Note that this will require downloading & building pfunit as part of the workflow (probably cached like we cache netcdf, etc.).

shr_orb should not require components to have a hack in their time manager for leap years

Currently, some subroutines in shr_orb_mod do not work correctly for leap years. As a workaround, CAM has the following in its time manager:

https://github.com/ESCOMP/CAM/blob/ce2e330303acc836a956d448f0e2e06d5c12821e/src/utils/time_manager.F90#L886-L898

and CTSM has something similar. The problems with putting this workaround in each component's time manager are that every component needs to be sure to implement this consistently, and the workaround ends up getting applied in situations where it shouldn't be. (The latter is what led us to discover the issue in CTSM; our plan is to only apply the workaround conditionally, so it is applied for some calls to get_curr_calday but not others.)

Based on yesterday's CSEG meeting, the proposed solution is to move the workaround into the appropriate subroutine(s) in shr_orb_mod itself, then removing this workaround from the components' time managers. So, within the components' time managers, Dec 31 on a leap year will give a calendar day that truly represents day 366, but then inside the shr_orb_mod subroutines this is converted to the previous day.

It is important that components remain consistent for (at least) two reasons:

(1) The calls to shr_orb_mod should be made consistently

(2) Surface components need to remain consistent with the atmosphere's (CAM's or DATM's) calculation of nextsw_cday; this is at least true for CTSM, where there is code like this:

https://github.com/ESCOMP/CTSM/blob/60b92b2711ff2e0a037d2049adf1a36a3a18ad48/src/cpl/nuopc/lnd_comp_nuopc.F90#L874-L875

so CTSM's internally-calculated calday should be done consistently with CAM's nextsw_cday.

So a proposed path forward is:

• Make the necessary changes in shr_orb_mod to translate too-large calendar days to day 365. This should be backwards compatible: if a component has already done the adjustment itself, then the new adjustment in shr_orb_mod won't do anything. This may result in changes for CICE (we're not sure), but shouldn't result in changes for CAM or CTSM.
• Change CAM, DATM, CTSM, CICE and any other needed components so that they no longer adjust calday internally, but instead rely on shr_orb_mod to do this adjustment.

If this plan isn't feasible for some reason, then an alternative is for @dabail10 to check CICE to confirm that it is doing its own adjustment of calday to be consistent with CAM.

@ekluzek @mvertens @gold2718 @cacraigucar @dabail10 @adamrher @olyson

coszen is off by a time step

Moving this over from the cime issue:

ESMCI/cime#3380

From some analysis by Sean Swenson, we see that the cosine solar zenith angle forcing is off by a time-step.

I think the problem is in shr_tInterpMod.F90 in shr_tInterp_getAvgCosz, where a time update happens and then the solar-zenith angle is calculated. I think that should be reversed the solar zenith angle is calculated and then the time-update happens.

This affects CLM standalone simulations for all forcing types (I compsets). But, also CORE_IAF and CORE2_IAF forcing (JRA and IAF C and G compsets as well as the DIAF compset).

mct_mod needs to be removed for cesm3

Dependancies in ctsm, scam, slim and cam/tools/mkatmsrffile

Document procedure for running Fortran unit tests

We should document the procedure for running the Fortran unit tests, either on the wiki or in a README file. See #17 (review) for the current process that I know of, but in #18 I suggest reworking this to make it easier – so if we plan on doing #18 soon then this documentation can wait until after that is done.

shr_string_listCreateField(), but for any range

I need to create a field list for a range of values that does not begin with 1. This doesn't work with shr_string_listCreateField(), which uses the input numFields as the top of the range and 1 as the bottom. I suggest implementing a new function, shr_string_listCreateField_range(), which takes inputs of the bottom and top of the range.

I've already written this up in a fork and will submit a pull request shortly.

github workflow needs work

The github workflow for this repository was using an mct test which has now been removed. We need to update the test build to use a nuopc workflow.

CIME path for Tools should be updated

Like in other externals this path should be updated. The path that was cime/scripts/Tools is now cime/CIME/Tools. The model is still building even with this though.

buildlib.csm_share:sys.path.append(os.path.join(_CIMEROOT,"scripts","Tools"))

Replace Henry's law data in seq_drydep_mod.F90 with input data

Move hard-wired data (effective Henry's law coeffs, molecule names and weights, etc.) in drydep_mod to an input file. This will allow more flexibility.

Add new functions to shr_orb

Moving over from cime:
Add new functions to shr_orb to calculate azimuth and incidence angles. This is needed for the CTSM hillslope model.

ESMCI/cime#3700

Define TINYVALUE in shr_kind_mod

As described here:

use ISO_FORTRAN_ENV, only : REAL64
TINYVALUE = tiny(1._REAL64)

ctsm lilac should use old shr_pio_mod.F90

The new version of shr_pio_mod.F90 used in cmeps uses NUOPC. Lilac does not use nuopc and
needs to continue to use the older version. This can be done by changing the line
https://github.com/ESCOMP/CESM_share/blob/main/buildlib.csm_share#L60 to be after line 71 iff lilac. That is if you are doing a lilac build you want share/src in the Filepath before components/cmeps/cesm/nuopc_cap_share.
But if you are doing a cmeps build you do not want this.

Changes in CLM elevation where SMB and Ts are computed (CESM_share portion)

Moving this from the issue that @whlipscomb opened in CIME (ESMCI/cime#2809): The first part of the solution will be implemented in CMEPS (ESCOMP/CMEPS#251), and the second part here:

In JG/BG runs, Marcus Lofverstrom has found spatial and temporal discontinuities in the surface mass balance (SMB) and surface temperature (Ts) downscaled to CISM from CLM elevation classes (ECs). These discontinuities are related to the different treatment of real (nonzero-area) and virtual (zero-area) glacier columns in CLM. For real columns in a CLM grid cell, SMB and Ts are computed at the mean elevation of the ice-covered CISM cells in that EC within the domain of the CLM cell. For virtual columns, CLM computes SMB and Ts at the midpoint elevation of the EC.

To take a concrete example: Consider EC 9, which includes the range from 2500 to 3000 m. If a CLM cell has no ice in this range, it computes SMB and Ts at 2750 m. Now suppose the CISM topography is evolving in a BG simulation, and at some point there is a CISM cell at elevation 2999 m in the domain of the CLM cell. Then CLM abruptly introduces a real EC at 2999 m, replacing the virtual EC at 2750 m. This can result in abrupt changes in the values of SMB and Ts downscaled to EC 10 cells in CISM, because the linear interpolation in the coupler now uses a lower bound of 2999 m instead of 2750 m for EC 9. The changes are usually more pronounced for Ts than for SMB, since Ts has a stronger elevation dependence.

The proposed solution has two parts. First, in module map_glc2lnd_mod.F90, we smooth the transition from virtual to real elevations in CLM. Let h_k* = the elevation where CLM computes SMB and Ts for EC k. The new formula for this elevation is

h_k* = (a_0*H_k + a_k*h_k) / (a_0 + a_k),

where H_k is the fixed midpoint elevation of EC k; h_k is the mean elevation of CISM cells in EC k; a_k is the fractional ice area in EC k, a_0 is a prescribed constant. For a_0 = 0, this reduces to h_k* = h_k, which is the previous approach. When we set a_0 to a small nonzero value, ~ 0.05, the transition in h_k* takes place gradually as a_k increases from 0 to a finite fraction of the cell area.

Marcus has tested a_0 = 0.01, 0.05 and 0.10. He finds that 0.01 is too small to fix the problem, while results with 0.05 and 0.10 are similar. For now we will choose a default value a_0 = 0.05.

The second part of the solution, in module glc_elevclass_mod.F90, is to redefine the midpoint elevation H for EC 10. This value is arbitrary, since EC 10 has no upper bound. We have been using H_10 = 3500 m, but we will change to H_10 = 3001 m. Then when EC 10 changes from virtual to real, e.g. by thickening of CISM cells that were in EC 9 but now are slightly thicker than 3000 m, there will not be a sudden change in the elevation where CLM computes SMB and Ts.

In tests to date, these changes have helped smooth out temporal and spatial discontinuities in downscaled CISM fields. They are answer-changing in CLM, although the changes are expected to be modest.

shr_orb_mod assumes a noleap calendar

This line

CESM_share/src/shr_orb_mod.F90

Line 704 in 4b31625

real (SHR_KIND_R8),parameter :: dayspy = 365.0_SHR_KIND_R8 ! days per year

and maybe others in shr_orb_mod assume a noleap calendar (searching for 365 shows some other comments mentioning this limitation, but I'm not sure if there is other code that assumes it or if it all comes down to that one parameter). It probably isn't too important to fix this, but I'm opening this for visibility and so that I can reference this issue from CTSM, where we need to have some workarounds in place for this limitation.

Make it easier to run the pfunit-based unit tests

Currently, to run the Fortran (pfunit-based) unit tests in this repository, I think you need to clone cime then checkout this CESM_share repository as an external of cime, then run all of the unit tests defined at the top level of cime. (See #17 (review) .) This is an awkward workflow, particularly since I feel like this CESM_share repository is the most important place to increase our unit test coverage, so it should be easy to write and run unit tests from here.

We do still need CIME to run the unit tests, but I think what would be better would be:

• Move the relevant pieces from cime's top-level CMakeLists.txt file into this repository
• The workflow would then be:
  • Checkout cime to anywhere on disk
  • From the top level of this CESM_share repository (or wherever the top-level CMakeLists.txt file will be defined), run /path/to/cime/scripts/fortran_unit_testing/run_tests.py --build-dir unit_tests_temp
• We can probably then delete cime's top-level CMakeLists.txt file, because I think the mct/cpl7 unit tests are no longer important to keep, and I don't think there are any other unit tests run from there (but we should check that)
• We can then also remove cime's scripts_regression_tests test, test_b_cime_f90_unit_tests, since it won't have anything to do anymore (but we should keep test_a_unit_test, which tests the general unit test infrastructure as defined in CIME).

Add a integer scatter/gather methods to shr_mpi_mod.F90

For CTSM we need an integer interface for the scatter/gather methods in shr_mpi_mod.F90.