E3SM-Project / E3SM-Project/E3SM

driver-mct: lnd->glc elevation-class downscaling demotes fields to single precision

Open Beginner friendly
#8,657 11 comments 0 reactions 0 assignees View on GitHub
MPAS-albany-landice PotentialBug
Dominant language
Fortran
Stars
440
Forks
481
Avg merge
4d 6h
Merged PRs (30d)
36

Description

While use claude to port land-ice to driver-moab, this precision issue was noticed. Should it be fixed? @matthewhoffman @jonbob

---

In `driver-mct/main/map_lnd2glc_mod.F90`, the per-elevation-class staging arrays used by the lnd->glc vertical downscaling are declared with a bare `real` (default single precision):

https://github.com/E3SM-Project/E3SM/blob/b9861201d9/driver-mct/main/map_lnd2glc_mod.F90#L350-L352

```fortran
real(r8), pointer :: tmp_field_g(:) ! must be a pointer to satisfy the MCT interface
real, pointer :: data_g_EC(:,:) ! remapped field in each glc cell, in each EC
real, pointer :: topo_g_EC(:,:) ! remapped topo in each glc cell, in each EC
```

and the horizontally mapped double-precision values are explicitly demoted into them with the bare `real()` intrinsic:

https://github.com/E3SM-Project/E3SM/blob/b9861201d9/driver-mct/main/map_lnd2glc_mod.F90#L418-L421

```fortran
call mct_aVect_exportRattr(l2x_g_temp, fieldname_ec, tmp_field_g)
data_g_EC(:,ec) = real(tmp_field_g)
call mct_aVect_exportRattr(l2x_g_temp, toponame_ec, tmp_field_g)
topo_g_EC(:,ec) = real(tmp_field_g)
```

As a result, every per-EC field value (`Flgl_qice`, `Sl_tsrf`) and per-EC topography coming out of the horizontal remap is rounded to ~7 significant digits before the vertical interpolation between elevation classes runs. The interpolation then mixes these single-precision class values with the r8 ice-sheet topography, so the downscaled fields sent to GLC (e.g. the surface mass balance seen by MALI) carry single-precision rounding, even though everything upstream and downstream of this routine is double precision. The driver is not compiled with `-fdefault-real-8`, so `real` here really is 4 bytes.

This looks like an oversight rather than an intentional memory optimization: the arrays are size `lsize_g * nEC`, so keeping them r8 costs little.

Suggested fix: declare `data_g_EC`/`topo_g_EC` as `real(r8)` and drop the `real()` demotion (or make it `real(..., r8)`). Note this would be answer-changing at the roundoff level for compsets with prognostic GLC coupling, so it would need a non-BFB PR label.

Contributor guide

Open the contributing guide

Research direction

Start in driver-mct/main/map_lnd2glc_mod.F90 around the staging-array declarations at lines 350-352 and the exports at lines 418-421; trace how the per-elevation-class values enter vertical interpolation. Done means the remapped fields and topography retain double precision through downscaling, with the resulting non-BFB roundoff change validated for prognostic GLC coupling.

Written by the indexing model from the issue text.

Assessment

Tech stack
fortran
Domain
hpc
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
67/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.