Add GenTS as an option for making time series files

[brianpm]

What this does

ADF turns model history files into time series files (one file per variable). Today it does that itself, by calling ncrcat. The CESM project is moving to a separate tool called GenTS for this same job.

This pull request adds a switch so ADF can hand that step to GenTS instead. It also lets ADF read time series files that GenTS already made somewhere else, so you don't have to make them again.

Nothing changes unless you ask for it. If you leave the new setting out of your config file, ADF behaves exactly as it does today.

How to use it

In your config file, under diag_basic_info:

ts_tool: gents

Use adf, or leave the line out entirely, to keep the current behavior.

There are a few optional settings, all of which can be skipped:

gents_all_vars: false        # true = make every variable in the file, not just the ones you listed
gents_nested_layout: false   # true = use the folder layout the CESM project archives
gents_slice_years: 10        # leave out = one file per variable covering all your years
gents_compression: zlib      # leave out = no compression
gents_compression_level: 2   # required whenever gents_compression is given

These are separate entries rather than one indented block, because ADF's config
reader only allows one level of indentation.

Why the change is small

I expected this to be a big job and it wasn't, for two reasons.

First, GenTS already names its files the same way ADF does, so nothing downstream needed to be taught a new naming scheme.

Second, if you point GenTS at the folder holding the history files, its output lands in one flat folder, just like ADF's own output. The nested folder layout only shows up if you ask for it.

I also worried that GenTS would leave out the extra pieces ADF needs to work with model levels (hyam, hybm, and friends). It doesn't. GenTS copies anything that doesn't change over time into every file, so those come along automatically.

One thing to watch out for

There is one real difference in behavior, and it's worth understanding.

When ADF makes the time series itself, it tucks a copy of surface pressure (PS) into every 3-D variable's file. GenTS doesn't do that. Because surface pressure changes over time, GenTS treats it as a variable in its own right and gives it its own file.

ADF can already find surface pressure in a separate file, but only if you have listed PS in diag_var_list. So if you use GenTS, add PS to your variable list. If you forget, ADF now stops with a clear message explaining why, instead of quietly dropping all your 3-D variables later on.

Installing GenTS

GenTS is optional and is deliberately not added to ADF's conda environment file. It needs newer versions of some packages than ADF currently pins, and changing those pins for everyone is a much bigger decision than this pull request.

Install it yourself, and please pin numpy:

pip install gents "numpy>=2.0,<2.3"

The pin matters. GenTS wants numpy 2.0 or newer. But numba, which gets pulled in while ADF computes climatologies, refuses anything newer than 2.2. If you just run pip install gents, you get numpy 2.5 and every climatology fails, while ADF still prints that it finished successfully. I hit this during testing and it took a while to spot.

If you install into a virtual environment on top of a conda environment, you will also need to set PROJ_DATA and ESMFMKFILE yourself, because virtual environments don't pick those up automatically.

A known problem you will see when testing this

While testing I found that every variable fails to be regridded, on this branch and on main alike.

The cause is a file-naming mismatch that has nothing to do with GenTS. The climatology step writes files named like mycase_cam.h0a_T_climo.nc, but the regridding step looks for mycase_T_*.nc. Those never match, so regridding quietly finds nothing. The run still says "completed successfully", because that step only prints a warning, which is why it's easy to miss.

This is already fixed in #435, which rewrites the regridding step to look up climatology files through the shared data-access code instead of matching filenames by hand.

I did not fix it here, to keep this pull request focused. I branched off main, which is why the problem shows up in my test runs.

Related issues

This addresses, or helps with, a few open issues:

• incorporate GenTS for time series generation #466 (incorporate GenTS) — this is that work.
• Time series generation requires NCO (ncrcat) #1 (time series generation requires NCO/ncrcat) — that issue lists three ways forward, the third being "a config option that lets the user choose". That is what this does. GenTS uses the netCDF library directly and never calls ncrcat, so with ts_tool: gents an ADF install no longer needs NCO for this step. The default path still uses ncrcat, so the requirement isn't gone, just avoidable.
• AMWG Tables for more than one time series file per variable  #161 (AMWG tables with more than one time series file per variable) — fixed. The table script had a note in it saying it could only handle one file per variable and skipped anything else. It now combines files that cover consecutive periods. Files covering overlapping periods are still skipped, with a clearer message, because combining those would double up the time axis. The example in that issue is a CMIP-style archive laid out exactly the way GenTS lays one out.
• Time Series creation timeout #442 (time series creation times out a terminal) — helped. That issue asks for something to be printed periodically so a session doesn't go silent and drop. GenTS prints a progress bar while it works, and this branch shows it when you're at a terminal and hides it when output is going to a log file.
• Allow option to compress generated netCDF files #22 (option to compress generated netCDF files) — partly. gents_compression compresses the time series files when GenTS is doing the work. Climatology and regridded files, and the default path, are unaffected.
• Capability to handle CMIP style time series file conventions for specified year range #179 (CMIP-style time series in chunks) — partly. Reading a variable that is split into consecutive chunks now works. Picking a subset of years out of a longer archive still does not.

Two related issues I looked at and deliberately did not fold in:

• ADF and LDF cannot create plots with specified date ranges if tseries are available for multiple time ranges #433 (multiple time ranges present at once) needs ADF to choose files by year, which is a real piece of work. This branch doesn't fix it, but it does make the failure obvious instead of confusing: you now get a message naming the variable and saying the periods overlap.
• Add functionality to derive variables if using pre-made time series files #297 (deriving variables from pre-made time series) needs a way to derive without reading history files, which is genuinely a separate change.

There is also #440 (make the time series directory instead of aborting), which suggests a change to the same function I touched. I'd push back on that one gently. That code only runs when cam_ts_done is true, which is the user saying "my time series already exist". If the directory isn't there, something is wrong with the configuration, and creating an empty directory just moves the failure somewhere less obvious. I think the clear error is the right behavior, so I left it alone — happy to be told otherwise.

Suggested order for merging

1. Taylor Diagram with observations and multi-case #435 first. It fixes the regridding problem described above. Merging it first means everything after it can be properly tested, including the map plots that are currently broken for everyone.
2. This one second. It shares three files with Taylor Diagram with observations and multi-case #435 (adf_dataset.py, adf_info.py, adf_utils.py), but the edits are in different parts of those files, so any conflicts should be small and mechanical. Merging it after Taylor Diagram with observations and multi-case #435 also means the GenTS option can finally be checked against working regridding, which is exactly where the surface pressure difference described above matters.
3. Cloud regime analysis #417 whenever convenient. It only overlaps with this branch in the example config file, and in a different section of it. It is independent of the other two.

Testing done

Run end to end on Casper, comparing cases b.e23_alpha17f.BLT1850.ne30_t232.098 and .093 (years 10-11).

• Ran the whole pipeline the old way (ts_tool: adf) to confirm nothing broke. Finished successfully.
• Ran the whole pipeline the new way (ts_tool: gents). Finished successfully.
• Compared the two. Both made 17 files with identical names. Opening the temperature file from each, the temperature data and the time values are exactly identical. Both runs produced the same 14 plots.
• Confirmed the vertical-coordinate pieces (hyam, hybm, and so on) really are inside the GenTS files, and that the "cannot be regridded" message never appears in either run.
• Confirmed the new error message appears if you forget to add PS.
• Read back a folder of GenTS files with cam_ts_done: true and no years given. ADF skipped making files and correctly worked out the years were 10 to 11.
• Confirmed ADF can also find files in GenTS's nested folder layout.
• All 55 unit tests pass, including 38 new ones.

Two things turned up while testing and are fixed here:

• The optional settings were originally written as one indented gents_options block. That silently doesn't work, because ADF's config reader only allows one level of indentation. They are now separate entries.
• Turning on gents_slice_years produced four files per variable and revealed that the AMWG table script skipped every variable in that situation (AMWG Tables for more than one time series file per variable  #161 above). Fixing that in turn exposed that the table statistics assumed data loaded from a single file. Both are fixed, and re-running the single-file case reproduces the old tables exactly, character for character.

One caveat on the comparison: because regridding is broken on main (see above), the matching plot counts only cover the plots that don't need regridding. The map plots weren't exercised for either option. That leg is worth redoing once #435 is in.

Review round (added after the first review)

An AGENTS.md-driven review of this branch is in the comments.
It found one blocker; a second, worse one turned up while fixing it. Both were in
derive_variable, and both are fixed here.

Blocker 1 - chunked constituents were truncated. derive_variable took
constit_matches[0], the first chunk only. With gents_slice_years: 10 over years
1-20, RESTOM came out covering years 1-10, named for that shorter span, and the AMWG
table then reported it alongside full-length variables with no warning. All of a
constituent's files are now opened together and the derived variable is written as a
single file spanning them, matching what the built-in back end produces. Constituents
whose own files overlap are refused, reusing ts_files_overlap.

Blocker 2 - TypeError on the aerosol path. load_dataset was handed a Path
where it needs a list; it starts with len(fils), and len(PosixPath) raises. This
only worked before the switch to find_ts_files because glob.glob()[0] returned a
string. It fires for any variable that is both derived and in aerosol_zonal_list -
SO4, SOA, BC, POM and DUST all qualify - which is why my test case (which only derives
RESTOM) never hit it. PMID and T now take the whole list, which also stops a
single-chunk PMID from silently NaN-ing a full-span product through time-axis
alignment.

Also addressed from the review:

• Cross-case contamination. The constituent, PMID and T patterns carried no case
  name, and find_ts_files falls back to a recursive search. With two cases under one
  tree covering adjacent (so non-overlapping) periods, mycase's RESTOM was built from
  both cases' data and written under the other case's directory under the other
  case's name: acase/acase.cam.h0a.RESTOM.000101-004012.nc, 480 times, instead of
  mycase/mycase.cam.h0a.RESTOM.000101-002012.nc, 240 times. Now anchored to the case
  name, falling back to the looser pattern so existing directories keep working.
• The new unit tests were skipping in CI. adf_utils imports numpy/xarray/pandas/
  geocat at module level, and the unit test workflow installs only PyYAML and pytest,
  so all ten tests were skipped and CI reported green. The three helpers moved to
  lib/adf_file_utils.py, which imports nothing but pathlib; adf_utils re-exports
  them so every caller is unchanged. Checked in a venv holding only pyyaml and pytest:
  10 skipped before, 33 passing and 0 skipped now.
• Recursive tree walks. A missing variable is normal, and
  get_climo_yrs_from_ts could trigger a full recursive walk per pattern per variable.
  find_ts_files gained a recursive keyword (default unchanged); the year search now
  sweeps flat first and makes a single recursive pass afterwards.
• NumPy-style Parameters/Returns on get_ts_case_config and
  create_time_series_gents (AGENTS.md 6.2), and comments on the two GenTS assumptions
  that read like bugs (pull_metadata mutating in place, primary_var being a GenTS
  internal).

Not changed here, deliberately. The review also flagged that
amwg_table._get_row_vals computes the standard error as std / n rather than
std / sqrt(n), and uses data.std() (ddof=0) where a sample standard deviation is
wanted. Both predate this PR. They change published table numbers, so they belong in
their own PR rather than riding along here.

Single-file constituents produce byte-identical filenames and contents, so the default
ts_tool: adf path is unchanged. pylint on adf_diag.py and adf_info.py is 9.81
against the 9.5 CI threshold (main is exactly 9.50).

Second review round

The fixes above were themselves reviewed, by an agent given only the diff and
AGENTS.md and none of the reasoning behind them. That was worth doing: it found
two blocking bugs that the first round introduced or left, both verified before
fixing.

Derived variables were lost for cases with several history streams. Derivation runs
once per configured stream, but derive_variable was never told which one, so the
constituent search matched every stream's copy — and two streams hold files with
identical dates. The overlap guard then rejected the constituent, and a case with
hist_str: [cam.h0a, cam.h0b] lost every derived variable. On the aerosol path PMID and
T from two streams went into open_mfdataset together, which cannot order datasets with
identical time coordinates, and the ValueError propagated out of create_time_series
and ended the run with a traceback (AGENTS.md §4.5 rules that out for framework code).

Verified against the round-one code and against this PR as originally submitted:

	as submitted	round one	now
RESTOM, two streams	file written	nothing written	file written
SO4 aerosol, two streams	TypeError	ValueError	file written

hist_str now reaches derive_variable from both back ends and is the first pattern
tried. Note the middle column: round one was a real regression on the first row.

The configured history stream could lose to the loose fallback. The flat-then-recursive
sweep in get_climo_yrs_from_ts checked every pattern flat, for every variable, before
recursing at all — so a stray flat file from another stream beat the configured stream's
file whenever that sat in a nested archive, contradicting the comment two lines above it.
Sweeping by pattern rank restores the precedence and keeps the performance win. Compared
old against new over 5100 combinations of 8 file layouts × 4 variable lists × 5 hist_str
settings:

identical result                                                4428
found vs not-found differs                                         0
new picks a non-configured stream where the old picked a
    configured one                                                 0
differ only in which *variable* supplied the dates               672

Those 672 are a deliberate consequence, recorded in a test: a flat match for a later
variable now beats a nested match for an earlier one. The method documents that it assumes
every variable covers the same dates, so which variable supplies them is not meaningful,
whereas which stream does is.

Also from this round:

• A silently all-NaN aerosol field. PMID and T are looked up with a fallback that can
  reach another case's files, and xarray aligns on time with an outer join — so a disjoint
  pair does not raise, it writes an all-NaN field that looks real. Reproduced writing SO4
  with nan_frac = 1.0; now refused with a message naming the variable.
• A mangled derived file name. The name came from a plain string replace of the
  constituent, so case bFSNTtest deriving RESTOM from FSNT produced
  bRESTOMtest.cam.h0a.RESTOM..., which AdfData.get_timeseries_file searches for by case
  name and can never find. Whole dot-separated tokens are substituted now.
• A support claim that was not true. Two comments said chunked CMIP-style archives
  work. They do not — the date token is the last dot-separated part of the stem, and a CMIP
  name has no interior dots, so it cannot be parsed and the files are refused. Refusing is
  right; claiming support was not. The comments now say only ADF/GenTS names have readable
  dates.
• lib/adf_file_utils.py is added to the linting workflow's testable_files set (it scores
  10.00/10), and no longer reports every variable as missing while walking a nested archive.

Tests. 38 new, up from the 16 after round one: test_adf_file_utils.py (13, and these
run in CI), test_adf_derive.py (9) and test_adf_info_climo_yrs.py (13). The latter two
import xarray so they skip in CI, which installs only PyYAML and pytest — noted in their
module docstrings, with moving the searches into adf_file_utils as the follow-up that
would make them CI-visible. Each set was run against the preceding commit to confirm it
fails there: of the 8 original test_adf_derive.py cases, 5 pass against round one and the
3 that fail are exactly the bugs above.

pylint on adf_diag.py and adf_info.py is 9.81 against the 9.5 threshold.

A note on speed

GenTS groups all the variables from one set of history files into a single task, so for a typical ADF run it works through them one at a time. In my test, 17 variables took about two minutes, where ADF's own approach spreads variables across processors. It's not wrong, just slower for this shape of job. Worth measuring before recommending GenTS for very large runs.

---

End-to-end runs on real data

Run on Casper against two deliberately dissimilar cases — different stream naming,
different era, different vertical grid:

• test: b.e30_alpha09e_m.B1850C_MTso_Gris_Marbl.ne30_t233_wgx3.389, cam.h0a, 93 levels, years 11–15
• baseline: ceresmip_amip02, cam.h0, 32 levels, years 2000–2004

Variables PS, TS, ICEFRAC, RESTOM, PRECT, Q — covering a derived variable, a 3-D
field needing vertical interpolation, and two fields with no obs entry.

run	regrid	PNGs	zonal	polar	Q levels	website	tables
model vs model, ts_tool: adf	13	280	70	140	200/850 hPa	yes	3 csv
model vs model, ts_tool: gents	13	280	70	140	200/850 hPa	yes	3 csv
model vs obs	6	200	50	100	200/850 hPa	yes	1 csv

The two back ends are equivalent. Same eight variables, identical file names,
identical time axes, and np.array_equal true on every variable's data — including
Q at (60, 93, 192, 288). The AMWG tables match character for character. The only
differences are the two documented ones: GenTS puts hyam/hybm/hyai/hybi in
every file, and the built-in back end embeds PS in the 3-D file where GenTS gives it
its own.

RESTOM derived correctly under both. The obs run correctly plotted only the four
variables that have obs entries and skipped ICEFRAC and RESTOM, which have none.
Zonal and polar plots were checked by eye, not just counted: Arctic ICEFRAC with
correct coastlines and more ice in the 1850 control than in AMIP; Q zonal showing the
tropical surface humidity maximum with Antarctic topography masked; obs TS against
ERAI at 288.55 vs 288.30 K, RMSE 1.34 K.

These runs needed #435. Two blockers, both in the regridding step and both fixed
there rather than here — see the merge order below, which they confirm:

1. On main the test-case climo lookup never matches, so all three configs produced
   zero plots while reporting success. Fixed by Taylor Diagram with observations and multi-case #435.
2. GenTS output tripped _determine_vertical_coord_type, which inspected the dataset's
   dimensions rather than the variable's, so 2-D fields were sent into hybrid
   interpolation and the run died with KeyError: 'lev'. Fixed in
   #435; it is that PR's function and this
   branch only exposes it.

Correction: the numpy pin above is out of date

The install guidance said to pin numpy>=2.0,<2.3 because numba rejects anything
newer. That was true of the numba installed when this was written, but the cap moves
with each numba release — numba 0.65.0 declares numpy<2.5,>=1.22. On the stack these
runs used (numba 0.65.0, numpy 2.4.3, gents 1.2.0, netCDF4 1.7.4) pip install gents
needed no dependency changes at all, and every climatology succeeded. The comment
and the ImportError message now describe the real constraint and say to check numpy
against numba after installing, rather than pinning a bound that may be wrong for your
environment.

[brianpm]

Tested with a derived variable (RESTOM), and found a bug that is not GenTS's

I wanted to check that the GenTS back end can handle a derived variable, so I ran
diag_var_list: [RESTOM, TS] on the example case (b.e23_alpha17f.BLT1850.ne30_t232.098,
years 10-11). RESTOM is not in the history files; ADF has to build it from FSNT and FLNT.

The GenTS side worked. It saw that RESTOM was missing, looked up its ingredients in the
variable defaults file, and made time series for FSNT and FLNT so that RESTOM could be
built from them afterwards. No changes were needed in adf_gents.py.

But the step that actually builds RESTOM crashed:

ValueError: When encoding chunked arrays of datetime values, both the units and dtype
must be prescribed or both must be unprescribed.
Raised while encoding variable 'time_bounds'

I then ran exactly the same thing with ts_tool: adf and it failed in the identical
way, so this is not a GenTS problem. It is a bug in the shared derivation code
(lib/adf_derive.py) that anyone deriving a variable would hit.

What was going wrong, in plain terms

To build RESTOM, ADF opens the FSNT and FLNT files together. Because it is opening more
than one file, xarray does not read the numbers right away -- it keeps them "lazy", as a
promise to read them later. That is fine for the big data arrays.

The trouble is a small variable called time_bounds, which records the start and end date of
each monthly average. When ADF goes to write the new RESTOM file, xarray has to turn those
dates back into numbers. It knows the units to write them in ("days since year 1"), because
it copied that from the time variable, but it has not been told what number type to use.
Recent versions of xarray refuse to guess for a lazy array, so instead of writing the file
they stop with the error above.

The fix is small: read those few date values into memory before writing. They are tiny (two
numbers per month), and once they are real numbers rather than a promise, xarray is happy to
write the file. Only date-like variables that are still lazy are touched; the actual data is
left alone.

A second, unrelated thing I fixed while I was in there

The derived variable was copying its description from the first ingredient it was made from.
So RESTOM -- the net radiation at the top of the model -- came out of the old code labelled:

long_name = "Net longwave flux at top of model"

which is the description of FLNT, not of RESTOM. Now the derived variable gets its own
description ("Net radiative flux at top of model (FSNT - FLNT)", or "Sum of ..." for the
variables that are simple sums), and a long_name in the variable defaults file will override
that if one is given.

The same copying problem also left the aerosol variables with the wrong units. Those get
multiplied by the density of dry air, which turns a mixing ratio (kg of aerosol per kg of air)
into a concentration (kg per cubic metre) -- but the file still said the old units. That is
corrected too.

Result

With those two fixes, both back ends produce RESTOM, and the numbers agree exactly:

	RESTOM file
ts_tool: gents	(24, 192, 288), max abs(RESTOM - (FSNT - FLNT)) = 0
ts_tool: adf	same shape and times, values bitwise identical to the GenTS file

Both files now carry long_name = "Net radiative flux at top of model (FSNT - FLNT)".
All 27 unit tests pass.

Since the fix is in lib/adf_derive.py rather than in the GenTS code, it will also fix
derived variables for people who never turn GenTS on. Happy to split it into its own PR if
reviewers would rather keep this one purely about the back end.

[brianpm]

A note on this review. This is a Claude-generated review, posted from my account but not written by me — we're trying out an AI review pass that follows the rules in the new AGENTS.md file, aimed at catching more errors and converging on consistent conventions. It reviewed my own PR here without me steering the findings. If you disagree with a convention it applies or think a finding is wrong, please say so — that feedback shapes both the process and AGENTS.md.

Summary. Adds a ts_tool: gents switch that hands time series generation to GenTS, plus a shared get_ts_case_config() so the two back ends read config identically, a find_ts_files/ts_files_overlap pair in adf_utils.py, chunked-file support in the AMWG table, and attribute fixes for derived variables. The design is clean, the default path is untouched, and the config documentation is unusually thorough. One blocking issue: gents_slice_years and derived variables interact badly — a derived variable silently ends up covering only the first slice.

Classification: framework (lib/) plus one analysis script and a config/defaults change. §4.1 does not apply (no new stage script); §4.2, §4.3, §4.5, §5, §6 do. adf_diag.py and adf_info.py are both in the CI-linted set (§3).

Blocking

1. gents_slice_years silently truncates derived variables. lib/adf_derive.py:186

constit_matches = utils.find_ts_files(ts_dir, f"*.{constit}.*.nc")
if constit_matches:
    constit_files.append(str(constit_matches[0]))   # first chunk only

derive_variable still assumes one file per constituent — the docstring at line 160 even says so. With gents_slice_years: 10 over years 1–20, FSNT and FLNT each get two files; only the first is opened, and derived_file = constit_files[0].replace(...) (line 213) names the output after that first chunk. The result is case.cam.h0a.RESTOM.000101-001012.nc — a RESTOM series covering half the requested period, with no warning.

Downstream this is silent: amwg_table finds one RESTOM file, ts_files_overlap correctly says no overlap, and the table reports RESTOM statistics over 10 years next to every other variable's 20. Nothing compares the file's span to the requested start_year/end_year.

This is a new interaction, not pre-existing debt: gents_slice_years is added by this PR, and the same PR teaches amwg_table to combine chunks while leaving derive_variable on [0]. Either pass the full constit_matches list into load_dataset (which already accepts a list, and ts_files_overlap is right there to guard it), or refuse gents_slice_years when constit_dict is non-empty. The first is closer to what the rest of the PR does.

Non-blocking

2. The rglob fallback can cross case boundaries in adf_derive.py. adf_utils.py:93 falls back to a recursive search when the flat glob misses. The patterns in adf_derive.py carry no case name — f"*.{constit}.*.nc" (186), "*.PMID.*" (247), "*.T.*" (258). In get_timeseries_file the pattern is anchored with {case}, so that path is safe, but a cam_ts_loc pointing at a tree holding more than one case could hand a derivation another case's constituent, silently. Anchoring these three patterns to the case name would close it.

3. rglob runs on every miss, and misses are normal. §4.2 treats a missing variable as an expected condition, and §5.5 flags per-variable re-reads. find_ts_files walks the entire tree whenever the flat glob finds nothing, and get_climo_yrs_from_ts (adf_info.py:875-882) loops that over len(hist_strs) + 1 patterns × every variable in diag_var_list until one hits. Pointed at a deep archive with an unlucky first variable, that is a lot of directory walking before the first useful work. Worth restricting the fallback to a bounded depth, or only attempting it once per directory rather than per pattern.

4. The 10 new unit tests will be skipped in CI, not run. test_adf_utils.py:27-30 guards the import and skipUnless-es the whole class. Per §3 the unit-test workflow installs only PyYAML and pytest, and adf_utils imports numpy, xarray, pandas and geocat.comp at module level (adf_utils.py:47-50) — so in CI _HAS_ADF_UTILS is False and all 10 tests skip silently, reporting green. They pass locally (27 passed here), but they are currently buying no CI protection for the two functions this PR most depends on. Both helpers need nothing but pathlib; moving them to a dependency-free module would make them genuinely testable, which matters because #161's chunk handling now rests on ts_files_overlap.

5. §6.2 docstrings on new public functions. AdfDiag.get_ts_case_config (adf_diag.py:326) and create_time_series_gents (adf_gents.py:117) are new and public, and both have prose-only docstrings with no Parameters or Returns sections. find_ts_files and ts_files_overlap do this correctly and are a good model. Minor: find_ts_files's Returns block gives a description where the type line should be.

6. Comment contradicts the code it labels. adf_utils.py:130-131 says "Second-to-last dot-separated token", directly above Path(fil).stem.split(".")[-1]. The date is second-to-last in the filename but last in the stem, which is what the code takes. The code is right; the comment will mislead the next reader.

7. Non-overlapping but non-contiguous files are combined silently. ts_files_overlap returns False for a gap (years 1–10 and 21–30), so amwg_table opens both and reports a mean, a sample count and a trend over a discontinuous record with no indication. Overlap is the dangerous case and is handled; a gap is at least worth a printed warning.

8. hfc.pull_metadata() is the one GenTS call not reassigned. adf_gents.py:213 — every neighbouring call follows hfc = hfc.something(...), this one relies on in-place mutation. It evidently works, but it reads as a bug and would break silently if GenTS ever returned a copy. A comment saying "mutates in place" (or reassigning if it returns self) would settle it.

9. _restrict_to_vars depends on a GenTS internal. adf_gents.py:110 indexes o["primary_var"] on the order objects. That is reaching past the documented surface into GenTS's data model; if the key is renamed the failure will be a KeyError deep inside generation. Worth a comment naming the GenTS version this was written against, given the module docstring already commits to a version-sensitive install.

10. PR description undercounts the tests. It says "All 21 unit tests pass, including 4 new ones"; there are 10 new tests and 27 total. Presumably written before the second commit.

Optional / follow-up (pre-existing, not this PR's job)

• amwg_table._get_row_vals computes the standard error wrongly. data_sem = data_std / data_sample (line ~377) divides by n, not √n, so the reported SEM and the 95% CI derived from it are both too small by a factor of √n. Separately, data.std() is ddof=0 where a sample standard deviation (ddof=1) is wanted. Both predate this PR, but the PR touches these exact lines, and the fix is small enough to be worth folding in or filing immediately — these numbers are published in the AMWG tables.
• derive_variable indexes find_ts_files(...)[0] for PMID (247) and T (258) with no emptiness check, so a missing file raises IndexError before the if not ds_pmid: guard below can run — that guard is unreachable. Pre-existing shape, carried over in the rewrite.
• lib/adf_derive.py still has no trailing newline.

Verified

• pylint improves. main scores exactly 9.50/10 on adf_diag.py + adf_info.py — sitting on the CI threshold — and this branch scores 9.81/10. Every remaining warning (adf_diag.py:456,613,673, adf_info.py:290,861,937) blames to earlier commits by other authors; the PR adds none.
• All 27 unit tests pass locally.
• Backwards compatibility holds: get_basic_info("ts_tool") or "adf" means an existing config with no ts_tool takes the unchanged path, and an unknown value fails through end_diag_fail per §4.5.
• diag_var_list returns a copy (adf_info.py:659), so check_derive extending the list inside _expand_derived_vars does not leak constituents into the ADF object — worth stating explicitly since it would be a nasty bug if it did.
• Both back ends now read config through get_ts_case_config and call derive_variable with identical arguments, so the "cannot drift apart" claim holds today.
• get_climo_yrs_from_ts's new hard error replaces what was previously an unbound-variable crash when var_list was empty — a strict improvement.

Not verified

• Everything on the GenTS side of the boundary. GenTS is not installed in this environment (ModuleNotFoundError: No module named 'gents'), so HFCollection/TSCollection semantics, whether pull_metadata mutates, whether slice_groups(start_year=...) aligns windows as the comment claims, and whether apply_path_swap("/hist/", ...) matches real archive paths were all read, not run. The end-to-end testing in the PR description covers this; a second pair of eyes with GenTS installed would be worth more than this review on those points.
• The regridding leg, which the PR description notes is broken on main and fixed by Taylor Diagram with observations and multi-case #435. The blocking finding above lands in derive_variable, which is upstream of regridding, so it is independent of that.
• Whether gents_nested_layout plus find_ts_files's rglob fallback compose correctly on a real CESM archive — that is exactly the configuration where finding CAM diagnostics only works with Python version 3.4 or later #2 would bite.

---

Finding #1 is the one to fix before merge. #4 (tests skipping in CI) is the one most likely to matter six months from now.

[brianpm-ucar]

Review notes

Reviewed against AGENTS.md. No blockers — this looks ready to merge.

This branch is based on c1c49c1, just before #435 landed, and touches three of the same files (adf_dataset.py, adf_info.py, adf_utils.py), so I merged current main into the head locally and ran everything against that combined state rather than against the branch alone. The merge is clean.

Checks on the merged state

• pytest lib/test/unit_tests — 71 passed.
• CI-shape run in a bare venv with only pyyaml and pytest — 34 passed, 26 skipped, no collection errors. The new tests guard themselves properly, and test_adf_file_utils.py runs in full because adf_file_utils.py imports nothing but pathlib, which is exactly what its module docstring sets out to achieve.
• pylint on the gated files — adf_info 9.77, adf_diag 9.78, adf_file_utils 10.00, all above the 9.5 threshold. adf_file_utils.py is correctly added to testable_files.
• py_compile clean on every changed file; all YAML parses.
• Style — zero trailing-whitespace and zero over-100-column added lines.

Verified specifically

• GenTS API surface, against the installed GenTS 1.2.0: every method adf_gents.py calls exists — HFCollection.include / pull_metadata / include_years / slice_groups, and TSCollection.copy / apply_path_swap / append_timestep_dirs / apply_overwrite / apply_compression / add_attrs / create_directories / execute.
• Optional-dependency handling is right: local import inside create_time_series, an actionable AdfError when GenTS is absent, deliberately kept out of env/conda_environment.yaml, and ts_tool: adf documented as the default so existing configs are unaffected.
• ds[var] is assigned before the aerosol-density block that reads it.
• self.data.load_dataset(...) now receives a list where it previously got a bare string and indexed a single character out of it — a real fix, not just a refactor.
• The get_climo_yrs_from_ts rank sweep does try every variable against the configured stream before falling back to the looser *h0* search.

Follow-up items — neither blocks this PR

• find_ts_files recursive fallback cost. get_timeseries_file, get_ref_timeseries_file and amwg_table all use the default recursive=True. A missing variable is a normal condition, and each miss triggers a full rglob of cam_ts_loc — harmless on a flat directory, but one whole-tree walk per missing variable per case on a large archive tree. get_climo_yrs_from_ts already solves this with its recursive=(False, True) sweep; the same treatment would suit the other callers.
• derive_variable says "Will use existing file" and then rewrites it. When the derived file exists and overwrite is false, the message is printed but there is no return, so the variable is recomputed and to_netcdf(..., mode='w') overwrites the file anyway. Pre-existing on main and untouched here — worth its own issue.
• Cosmetic: the comment in _restrict_to_vars notes it is "worth pinning down which GenTS version a failure came from", but nothing records it. importlib.metadata.version("gents") works (returns 1.2.0) if you want that in the error path; gents.__version__ does not exist.