Derive the EAMxx source vertical grid file at run time - #864
Draft
chengzhuzhang wants to merge 2 commits into
Draft
Derive the EAMxx source vertical grid file at run time#864chengzhuzhang wants to merge 2 commits into
chengzhuzhang wants to merge 2 commits into
Conversation
zppy hardcoded an L128v1 vertical coordinate file as ncremap --vrt_in whenever prc_typ == 'eamxx'. E3SM PR #8692 makes L128v4 the default EAMxx grid, and because the level count is unchanged at 128, ncremap accepts the v1 file against v4 input and silently interpolates with the wrong coefficients. Derive the file from the run's own output instead: ncks the hybrid coefficients out, then append P0 with ncap2. This is grid-agnostic, so L72, L128v1, L128v4 and any future EAMXX_VGRID work with no staged file. [ts] derives from the raw history file, so it does not depend on extra_vars; [e3sm_to_cmip] derives lazily from the file it is about to remap, since its loop usually no-ops for EAMxx. An explicit vrt_in_file still takes precedence and skips the derivation. Fixes #863 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGCrzkffuiQ1Y5PR9FGv7C
EAMxx does not write P0 today, which is why the staged vert_L128.nc had one appended by hand and why the derivation synthesized the reference pressure. Later EAMxx versions will write it, so take P0 from the source when it is there and fall back to appending 100000.0 only when the extraction fails. Verified against real files: a source without P0 yields the synthesized 100000, a source carrying P0=99999 keeps 99999, and a source missing the hybrid coefficients still fails loudly with the NCO error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGCrzkffuiQ1Y5PR9FGv7C
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #863.
Problem
zppy hardcoded an L128v1 vertical coordinate file (
e3sm_to_cmip_data/grids/vert_L128.nc) asncremap --vrt_inwheneverprc_typ == 'eamxx'. E3SM#8692 makes L128v4 the default EAMxx grid, and because the level count is still 128,ncremapdoes not error on v4 input — it interpolates with v1 coefficients and emits silently wrong pressure-level data intots_vrt_remap/andcmip_ts/.Setting
vrt_in_fileexplicitly already worked as a user override, so this is a bad default rather than a missing capability. The problem is that a user who does not know to set it gets no error.Change
Derive the source vertical grid from the run's own output rather than shipping a grid-version-specific data file:
(the fallback is the operation recorded in
vert_L128.nc's ownhistoryattribute). This is grid-agnostic — L72, L128v1, L128v4 and any futureEAMXX_VGRIDwork with no staged file and no zppy release.ts.bashderives from the raw history file (head -n 1 input.txt), which always carrieshy*, so the[ts]path does not depend onextra_vars. Onlypsremains load-bearing there, for--ps_nm. Failure gets its own status code 7.e3sm_to_cmip.bashderives lazily, inside the loop, from the file it is about to remap, guarded so it runs once. Eager derivation would have been a regression:interp_varsdefaults to EAM names, so that loop usually remaps nothing for EAMxx, and a job that never callsncremapmust not fail on a missing derivation. Status code 5, with a message naming[ts] extra_varsas the fix.vrt_in_file, so an explicitvrt_in_filestill takes precedence and short-circuits the derivation entirely.vrt_remap_file/cmip_plevdata(the--vrt_outplev19 target) are untouched — the target grid is unaffected by this.Docs,
default.inicomments, and theexamples/post.v3.eamxx.cfgheader no longer claim avert_L128.ncdefault. No references to that file remain anywhere inzppy/,docs/, orexamples/.Tests
New
tests/test_vertical_remap.pyrenders both templates and asserts three cases: derived (novert_L128, derivation emitted), user-suppliedvrt_in_file(path wins, no derivation), andprc_typ = eam(no--vrt_in/--ps_nmat all). 96 unit tests pass; pre-commit clean.Verification
Checked against the L128v1 EAMxx case from
examples/post.v3.eamxx.cfg(ne256pg2 ... F20TR-SCREAMv1, ne30pg2 monthly output, 1995-1999), whosets/andts_vrt_remap/output from the old staged-file code is still on scratch.vert_L128.ncbit-for-bit.hyai,hyam,hybi,hybmandP0(double, 100000.0) are bit-identical, whether derived from the raw history file (the[ts]path) or from the split per-variable ts file (the[e3sm_to_cmip]path).levtop/bottom match at 2.5802608 / 998.49646 hPa. The derived file additionally carriesilev, whichnckspulls in as a coordinate;ncremapdoes not use it.ncremapoutput is bit-identical. Samencremapline on a two-month subset ofT_mid_199501_199912.nc, derived vs staged--vrt_in:maxabsdiff = 0.0over all (2, 19, 180, 360) values, with no change in missing-value count. The same holds against thets_vrt_remap/T_mid_199501_199912.ncthat the old code wrote in July, so this is a real before/after comparison and not just self-consistency within one session.P0is genuinely required, not assumed by NCO. WithP0stripped from an otherwise identicalvrt_infile,ncremapfails:ERROR Failed to vertically interpolate. cmd_rgr[0] failed. Thencap2step is load-bearing, hence the status check around it.P0handling is future-proof. EAMxx writes noP0today, but later versions will, so the derivation asks for it first and synthesizes 100000.0 only when that extraction fails. Checked against real files: a source withoutP0gives the synthesized 100000; a source carryingP0 = 99999keeps 99999 rather than being overwritten; a source missing the hybrid coefficients still fails loudly with the NCO error and trips the status code.levtop differs fromvert_L128.nc's 2.58 hPa. Nothing to test against yet.Draft pending that last item.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NGCrzkffuiQ1Y5PR9FGv7C