DatasetChunker stops write slices at the 4MB minimum instead of filling toward the max

[rhaegar325]

Summary

Every daily variable was writing ~4MB Dask slices regardless of the 128MB
maximum, because calculate_chunk_size_for_variable() only grew the time
chunk far enough to reach target_chunk_size_mb (the 4MB minimum) when
a single time step was smaller than it. For atmos.ta.tavg-p19-hxy-air.day
that meant 730 write slices per 4-year output file; each slice costs a
serial graph-cull / client.compute / gather round trip in the main process,
so the Dask workers idled while that round trip repeated.

Fixed by batching time steps toward max_chunk_size_mb in both regimes,
which collapses the two branches into one expression.

Measured on real ACCESS-ESM1-6 data, PBS compute nodes, ta p19 daily,
years 101-104, 7 workers / 128GB:

	Before	After
Write slices	731 x 4.01MB	24 x 126.25MB
submit (serial, main process)	48.9s	0.9s
Write loop total	130.9s	9.0s (14.5x)
Job walltime	8:23	6:42 (-20%)
Job CPU time	15:28	11:01 (-29%)

Output is bit-identical: all 8 variables in the file compared element-wise,
including the full 1461x19x144x192 ta array.

Root cause: a storage-chunk rule applied to Dask task size

DatasetChunker arrived (3b8d03a0, 2025-12-09, PR #137 "Chunking of data to
follow new CMIP7 directives") with only a 4MB minimum and no upper bound
at all, framed as an HDF5 storage-layout rule — CMIP7 mandates >=4 MiB storage
chunks. With no ceiling to grow toward, "stop once you reach 4MB" was the
entire algorithm, not a decision against larger chunks.

But the value never set a storage chunk. chunksizes was passed briefly in
429451f2 (2025-12-03) via a to_netcdf/encoding write path and removed the
same day in a56ea2ad when the write was rewritten to the manual two-phase
createVariable path. DatasetChunker landed six days later into a code path
that no longer had storage chunking, so the 4MB floor has only ever controlled
Dask task size while carrying a storage-layout rule's name.

Two later PRs each fixed one half and left the other as found:

• edcd014b (fix: bound Dask spatial chunk sizes #555, 2026-07-27) added max_chunk_size_mb=128.0 purely as a
  shrink ceiling for oversized spatial slabs, and documented in the docstring
  that these are not storage chunks. It never touched the growth side.
• 7e5efa2f (DatasetChunker never batches multiple time steps when a single step already meets the minimum target #603, 2026-08-10) fixed growth for the step size >= target
  regime (atmos.cl). Its own table lists step size < target as
  "unchanged: grows toward the 4MB target" — left alone because it satisfied
  the original "at least 4MB" rule. Daily variables are almost all in that
  regime (ta p19 is 2.02MB/step, tas is 0.11MB/step).

The CMIP7 >=4 MiB directive is real and still enforced — by cmip7repack -d
(minimum 4194304). This change does not touch output storage chunks.

Change

src/access_moppy/base.py — both branches now use the same expression:

time_chunks = max(1, max_target_elements // other_elements)
time_chunks = min(time_size, time_chunks)

The now-unused local min_target_elements is removed. Note that
self.target_chunk_size_bytes is left in place but has no remaining reader;
target_chunk_size_mb is still validated against max_chunk_size_mb and
logged, so the constructor's public surface is unchanged.

Two diagnostic log lines were added alongside, because the investigation could
not attribute wall time without them and neither could the next one:

• _write_dask_slices reports slice count, slice size and the
  submit / wait / write split — only wait is parallel worker time.
• _repack_cmip7_output reports its elapsed time — cmip7repack is a
  single-threaded external process, so its share of wall time is time no
  worker can be busy for.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.8%. Comparing base (270d15e) to head (1ca856a).

Additional details and impacted files

@@          Coverage Diff          @@
##            main    #693   +/-   ##
=====================================
  Coverage   78.7%   78.8%           
=====================================
  Files         41      41           
  Lines       9061    9070    +9     
  Branches    1693    1692    -1     
=====================================
+ Hits        7134    7143    +9     
  Misses      1592    1592           
  Partials     335     335           

Flag	Coverage Δ
unit	78.8% <100.0%> (+<0.1%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.