Skip to content

Update variables for v3 ELM#334

Merged
chengzhuzhang merged 8 commits intomasterfrom
update_land_var_v3
Mar 24, 2026
Merged

Update variables for v3 ELM#334
chengzhuzhang merged 8 commits intomasterfrom
update_land_var_v3

Conversation

@chengzhuzhang
Copy link
Copy Markdown
Collaborator

@chengzhuzhang chengzhuzhang commented Dec 16, 2025

Description

● Brief Summary of Changes

Variables Added/Modified in handlers.yaml

Land Variables (CMIP6_Lmon.json) - 12 variables

  • burntFractionAll - Burnt area percentage from fire
  • cCwd - Coarse woody debris carbon pool
  • cLeaf - Leaf carbon pool
  • cRoot - Total root carbon (fine + coarse live + coarse dead)
  • nppLeaf/nppRoot/nppWood - NPP allocation to plant components
  • rGrowth/rMaint - Growth and maintenance respiration
  • tran - Transpiration (modified to exclude soil evaporation)

Land Ice Variables (CMIP6_LImon.json) - 7 variables

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

If applicable:

  • New and existing unit tests pass with my changes (locally and CI/CD build)
  • I have added tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have noted that this is a breaking change for a major release (fix or feature that would cause existing functionality to not work as expected)

@chengzhuzhang chengzhuzhang marked this pull request as ready for review December 16, 2025 22:11
@chengzhuzhang
Copy link
Copy Markdown
Collaborator Author

@tomvothecoder Somethow I got inconsistent mypy results again CI/CD vs my local pre-commit run, which passed. This is not urgent, but when you have a change, can you help to fix the test?

@acme-y9s The proposed new v3 variables are supported by this PR. (I removed several variables only present in CMIP5 but not in CMIP6, e.g. cMisc, cWood, fLuc. burntArea is renamed to burnAreaFractionAll from CMIP5 to CMIP6. I have a example script to process and cmorize these variables as well.

Could you please inspect the sample CMIP files at LCRC under: /gpfs/fs0/globalscratch/ac.zhang40/e2c_tests/CMIP6/CMIP/E3SM-Project/E3SM-1-0/piControl/r1i1p1f1/
Please review if the values (including signs), distribution are reasonable. Thanks!

Copy link
Copy Markdown
Collaborator

@tomvothecoder tomvothecoder left a comment

Choose a reason for hiding this comment

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

Review of my latest commits. Should be good to go now.

Comment on lines +805 to +824
moc = xarray.concat(mocSlices, dim="lat") # type: ignore[arg-type]
moc = moc.transpose("Time", "nVertLevelsP1", "lat")

# average to bin and level centers
moc = 0.25 * (
moc[:, 0:-1, 0:-1] + moc[:, 0:-1, 1:] + moc[:, 1:, 0:-1] + moc[:, 1:, 1:]
moc_avg = (
0.25
* (
moc[:, 0:-1, 0:-1] # type: ignore[index]
+ moc[:, 0:-1, 1:] # type: ignore[index]
+ moc[:, 1:, 0:-1] # type: ignore[index]
+ moc[:, 1:, 1:] # type: ignore[index]
)
)
moc = moc.rename({"nVertLevelsP1": "depth"}) # type: ignore
binCounts = xarray.DataArray(binCounts, dims=("lat")) # type: ignore
moc = moc.where(binCounts > 0) # type: ignore
moc_avg = moc_avg.rename({"nVertLevelsP1": "depth"})
binCounts = xarray.DataArray(binCounts, dims=("lat")) # type: ignore[assignment]
moc_masked = moc_avg.where(binCounts > 0) # type: ignore[operator]

_compute_dask(moc, showProgress, "Computing {} MOC".format(regionName))
_compute_dask(moc_masked, showProgress, "Computing {} MOC".format(regionName))

mocs[regionName] = moc
mocs[regionName] = moc_masked
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

mpas.py changes to make mypy happy

for path in paths:
ds = xr.open_dataset(path, decode_timedelta=True)
data_vars = list(ds.data_vars.keys())
data_vars = [str(var) for var in ds.data_vars.keys()]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

mpas.py changes to make mypy happy

Comment on lines 384 to +385
with pytest.raises(KeyError):
rsuscs(xr.Dataset())
rtmt(xr.Dataset())
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fixed incorrect function reference in test.

# Test when required variable keys are NOT in the data dictionary.
with pytest.raises(KeyError):
rsuscs(xr.Dataset())
tran(xr.Dataset())
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fixed incorrect function reference in test.

Comment on lines 388 to +394
def test_tran():
ds = xr.Dataset(
data_vars={"QSOIL": _dummy_dataarray(), "QVEGT": _dummy_dataarray()}
)
ds = xr.Dataset(data_vars={"QVEGT": _dummy_dataarray()})

result = tran(ds)
expected = xr.DataArray(
dims=["lat", "lon"],
data=np.array([[0, 2, 4], [0, 2, 4], [0, 2, 4]]),
data=np.array([[0, 1, 2], [0, 1, 2], [0, 1, 2]]),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Updated test_tran() to get it passing for v3 formula code change.

@chengzhuzhang
Copy link
Copy Markdown
Collaborator Author

@acme-y9s Xiaoying, I'm trying to wrapping up this PR in preparation for a new e3sm_to_cmip release. Do you have a chance to review the variables mentioned here. We are aiming to merge PRs by end of March. Thanks!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates E3SM-to-CMIP CMOR handlers and formulas to support additional ELM v3 land (Lmon) and land-ice (LImon) variables, including adjusting the definition of tran to exclude soil evaporation.

Changes:

  • Add new CMIP6 land and land-ice variable handler entries (and corresponding formula functions where needed).
  • Update tran handler/formula and align unit tests with the new definition.
  • Minor robustness/type-related adjustments in the runner and MPAS MOC computation, plus a new debug run script.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
e3sm_to_cmip/cmor_handlers/handlers.yaml Adds new Lmon/LImon handlers and updates tran mapping/formula.
e3sm_to_cmip/cmor_handlers/_formulas.py Updates tran formula and adds new formula functions for newly introduced handlers.
tests/cmor_handlers/test__formulas.py Fixes/updates unit tests for rtmt KeyError behavior and tran output/inputs.
e3sm_to_cmip/runner.py Ensures discovered dataset variable names are collected as str.
e3sm_to_cmip/mpas.py Refactors MOC time series computation variables for clearer typing/flow and computes the masked/averaged result.
scripts/debug/334-new_v3_land_handlers/run_v3_land.sh Adds a debug script for running v3 land handler CMORization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@chengzhuzhang chengzhuzhang merged commit da26c3d into master Mar 24, 2026
7 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in E3SM to CMIP Development Mar 24, 2026
@chengzhuzhang chengzhuzhang deleted the update_land_var_v3 branch March 24, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

[Feature]: add land variables for v3

4 participants