Domain test matrix: sweep tests across 17 lat/lon topologies - #274
Draft
manishvenu wants to merge 17 commits into
Draft
manishvenu wants to merge 17 commits into
manishvenu wants to merge 17 commits into
Conversation
MOM6/CESM's FMS-based calendar reader accepts "gregorian" but not the also-CF-valid "standard" -- CrocoDash-generated cases were crashing at OBC init / CESM CALENDAR setup with a FATAL calendar-attribute error (#261). Calendar gains a third field (mom6) alongside the existing cf/cesm ones, so a product's CF-correct "standard" time-decode calendar can stay separate from the literal string regridded output files must carry. obc.py's OBC regrid step now stamps mom6_calendar ("gregorian"/"noleap") instead of cf_calendar ("standard"/"noleap") on its output.
CrocoDash#261: extract_forcings/config.json's "conditions" entry duplicated start_date/end_date under both inputs and outputs (outputs was a pure passthrough of inputs), and only had a single "step" value that fed regrid chunking -- the GET/retrieval phase silently ignored it and always fetched the whole date range in one request regardless of what "step" said. Drop the redundant start_date/end_date ConfigOutputParams; driver.py and recipe.py now read them from conditions["inputs"] instead. Replace the single "step" output with get_step_days/regrid_step_days so both process_obc_conditions() chunk params (which already existed independently) are actually wired to config.json.
CrocoDash#261: users never edit this file directly -- only config.json is meant to be touched in the LDW -- so prefix it with an underscore to signal that. FILENAME lives in one place (case_state.py); every other reference was via case_state.write()/read() and just followed.
Split extract_forcings/obc.py and the old initial_condition.py into model-agnostic GET/REGRID[/MERGE] engines (obc.py, ic.py) plus a MOM6-specific callback layer (mom6.py) supplying the regrid step via regional_mom6's Segment/experiment classes. driver.py's ic/bc dispatch now goes through mom6.process_mom6_ic/process_mom6_obc. Bumps the rm6 submodule pointer to pick up the newer experiment/segment API (Grid/VGrid objects passed directly via hgrid_type/vgrid_type instead of hgrid_path/vgrid_path strings; rotational_method removed from regrid_velocity_tracers), which also required updating tides.py and case.py's expt property to the new construction pattern. Ported from work already done on the cice branch; this pulls out just the generic engine split, without the CICE/WW3-specific pieces, so it can land as its own layer beneath the forcing-configurator refactor.
Splits ForcingProduct's velocity/tracer/SSH metadata out into VelocityTracerForcingProduct -> MOM6ForcingProduct, so a forcing product's model-specific regridding contract is explicit rather than folded into one generic class. Re-parents GLORYS and MOM6_OUTPUT onto MOM6ForcingProduct. Adds REFERENCE_OCEAN, a fast deterministic synthetic ocean IC/OBC product (no network/credentials required) for tests and demos that exercise the MOM6 IC/OBC pipeline without real data access. Ported from the cice branch, split down to just the MOM6-relevant piece -- CICEForcingProduct/WW3ForcingProduct and the REFERENCE_ICE/ REFERENCE_WAVES products land with the cice/ww3 branches instead.
…tor-obc-ic Kept our stack's rm6 submodule pointer (1c48b714) rather than main's side's older, diverged one -- ours has commits this stack already depends on (Segment ocean_side rename, mom6_forge._supergrid rotation, top-level Grid/Topo/VGrid exposure) that main's pointer doesn't have. The "ignore degenerate source cells" OBC regridder fix on main's side needs separate follow-up to confirm whether it's already covered on our rm6 lineage. Manually ported the two small non-rm6 fixes main brought to files this branch already rewrote: - ic.py: log data_access_function._how_to_use, same as initial_condition.py (now deleted, superseded by ic.py/obc.py) used to. - obc.py: simplified chunk-end date-slicing to plain date-string slicing (pandas partial-string indexing already covers the whole calendar day), dropping the old +23:59:59 workaround. raw_data_access/datasets/mom6_output.py deleted (superseded by cesm_ocean_output.py, which arrived via a clean auto-merge of raw_data_access/base.py's registry).
REFERENCE_OCEAN previously only had tracer_var_names (temp/salt), so any MARBL-enabled compset (%MARBL-BIO) hard-failed in write_metadata() with "This product does not have marbl tracer var names". Add the same marbl_var_names set CESM_POP_OUTPUT uses, identity-mapped, and generate a small-constant placeholder variable per tracer in get_reference_ocean_data so downstream IC/OBC regridding has real data to read.
Until now the whole suite exercised a single grid -- panama1, a 4x3 deg
box at 278E/7N -- which sits in the easiest corner of the parameter
space: northern hemisphere, mid-latitude, 0-360 convention, no seam,
axis-aligned, rectangular. Every seam and high-latitude bug found so far
(GLORYS antimeridian slicing, the mom6_forge dx metric bug, the polar
get_bounding_boxes fixes) was found by hand on a one-off case, not by
the suite.
Add a catalog of 17 domain specs covering the lat/lon topologies
CrocoDash can be handed, plus a pytest layer that parametrizes any test
over it. Writing a regression test for a new seam bug is now a one-line
`def test_x(domain_grid)`.
tests/fixtures/domains.py holds the whole framework -- dataclass,
catalog, selection, fixtures -- in one file, auto-discovered by the
existing fixtures/*.py glob. Adding a domain is adding one row.
Selection, in precedence order:
pytest tests/domains # cheap tier (6 domains)
pytest tests/domains --all-domains
pytest tests/domains --domain-tags=seam,polar
pytest tests/domains --domains=arctic_cap
Three test modules:
test_grids.py pure grid invariants; metrics finite and
positive; supergrid roundtrip; 0-360 vs
-180/180 convention pairs agree
test_bounding_boxes.py Grid.get_bounding_boxes, the chokepoint
between a domain and the data fetched for it
test_forcing_pipeline.py end-to-end against REFERENCE_OCEAN; asserts
no output is entirely NaN, the characteristic
symptom of a bbox bug that completes silently
Three mom6_forge bugs are pinned as strict xfails so their fixes
announce themselves: the dx/dy sign and antimeridian wrap (NCAR/
mom6_forge#113), the tarea quadrant double-count (NCAR/mom6_forge#126),
and the inflated bounding-box longitude span on a rotated domain
crossing the dateline. cyclic_global is pinned too -- get_bounding_boxes
asserts against cyclic grids outright, a known v1 limitation.
Also replace --runslow with the standard `slow` marker. It reimplemented
what -m "not slow" already does. Slow tests now run by default, so CI
gains -m "not slow" to keep its current behaviour.
CrocoDash_case_factory takes an optional grid_topo_vgrid, defaulting to
the session fixture as before. environment.yml declares pyproj, which
Grid.from_projection and Grid.from_center need and which arrived only
transitively.
|
📄 Preview your docs here: |
The end-to-end sweep built a real CESM Case per domain and ran configure_forcings/process_forcings against REFERENCE_OCEAN. That is ~3 minutes for the six-domain cheap tier and ~10 for --all-domains, it needs a real CESM root so it cannot run on a laptop, and CI's -m "not slow" does not deselect `workflow`, so it would have landed in CI by default. The cost would be worth it if the tier caught what the cheap tests cannot, but it does not: REFERENCE_OCEAN synthesizes data for whatever bounding box it is handed rather than slicing a fixed global dataset, so a wrong bbox still produces clean non-NaN output. Every seam and polar pipeline test passed for that reason, not because the boxes were right. What it actually proved was "the pipeline does not crash on this topology" -- worth having, but that is what crocontainer is for. Bounding-box correctness is asserted directly in test_bounding_boxes.py, which is where the value was all along. Removes test_forcing_pipeline.py and the scaffolding that existed only for it: the domain_case and domain_grid_topo_vgrid fixtures, configure_domain_forcings() and its cvars-snapshot dance, and the grid_topo_vgrid argument added to CrocoDash_case_factory. All of it is recoverable from 6666484 when the crocontainer follow-up needs it.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
recipe.build_grid handled two of the four ways to make a Grid: Grid(**cfg), and Grid.from_supergrid when the config carried supergrid_path. from_projection and from_center were unreachable, so a polar cap or a rotated domain could not be expressed in a YAML config at all -- not from the CLI, not from crocodash create, not from the MCP server. Four of the five most interesting rows in the domain catalog were unreachable from YAML for that reason. Dispatch on grid.type, mirroring what build_topo and build_vgrid already do with topo.source.type and vgrid.type. grid.type carries two families of value. from_file, from_projection and from_center name the alternative constructors and are stripped before the remaining keys are forwarded. uniform_spherical and rectilinear_cartesian are the supergrid flavours Grid.__init__ has always taken, and are passed through to it untouched -- Grid(**cfg) already forwarded `type`, so rectilinear_cartesian may already appear in configs in the wild and must not be swallowed by the new dispatch. A config with no type at all behaves exactly as before. DomainSpec.builder is now spelled with the same values as grid.type, and DomainSpec.to_grid_config() emits the config block, so test_domain_is_expressible_as_a_recipe_config can build every catalog domain both ways and require the coordinates to match. That test is what caught the type-key collision above: the cartesian_rectilinear row carries its own type=, which an earlier version of this dispatch clobbered. case_to_yaml is unchanged -- it always writes supergrid_path, so a grid round-trips through its written supergrid file however it was built.
manishvenu
marked this pull request as draft
August 20, 2026 16:15
regional_mom6.py imports ruamel.yaml at module scope, so nothing in the suite can even be collected without it -- tests/conftest.py auto-globs fixtures/, which pulls in the whole CrocoDash -> rm6 chain. It was arriving only transitively, which held right up until an environment rebuild resolved without it. Same class as the pyproj declaration above.
Three of these tests documented open mom6_forge bugs as strict xfails. That works only against the checkout they were written for: point CrocoDash at a mom6_forge carrying NCAR/mom6_forge#113 and #126 and the suite reports 22 failures, 20 of them XPASS(strict) -- the fixes landing read as breakage. Ask the installed mom6_forge what it does instead. Two probes build a throwaway 4x4 grid (and one 4x4 polar cap) and check the behaviour directly, so tarea_matches_supergrid_quadrants and grid_metrics_positive_and_finite xfail only where the bug is actually present. lru_cache keeps it to one probe per session. The bounding-box test needed a different answer, because #113 does not fix that bug -- it widens any >180-degree raw span to the full range rather than narrowing it, to stop lon_max landing exactly on 180.0 and being collapsed to -180.0 downstream. Narrowing would need the box to carry a wrapped range (lon_min > lon_max) that every consumer would have to understand. So the assertion is now the sharper one it should have been: a domain whose true angular span is modest must not get a near-global bounding box. Domains that genuinely span most of the globe are exempt, since no box can be too wide for them and there is nothing useful left to assert. That separates the polar caps -- 358.6 degrees, legitimately -- from rotated_on_dateline, which is 5.2 degrees wide and reports 360. The old assertion happened to pass on the caps by comparing against their own raw span, which was accidental rather than meaningful. Verified both ways on the full 17-domain sweep: pinned mom6_forge 209 passed, 6 skipped, 27 xfailed #113 + #126 229 passed, 6 skipped, 7 xfailed No failures either way, so bumping the mom6_forge pointer is now a no-op for this suite rather than a 22-failure event.
process_forcings hangs indefinitely in _regrid_obc_chunk's staging to_netcdf, at a rate around one case in four on a CPU-constrained machine. Three consecutive crocontainer domain sweeps wedged on 1, then 3, then 4 of 16 domains, with disjoint sets each time -- every one of the 16 domains passes on some run and fails on another, so the grid topology is not the trigger. The faulthandler stack is identical in every case: the main thread parked in dask.local.queue_get waiting on a worker inside HDF5 that never returns. Wrapping the write in dask's synchronous scheduler removes the thread pool the deadlock needs, and leaves the chunk-by-chunk memory profile unchanged (unlike .load(), which would also fix it but pull each chunk fully into memory). This is the same guard, for the same reason, as _download_initial_condition in ic.py. _merge_boundary in obc.py gets it too. That write has not been observed to wedge, but open_mfdataset makes it dask-backed and it is the identical construction two functions from a known deadlock. Left alone: mom6.py's write_regridded_ic_file, whose open_dataset is eager and never builds a dask graph, and bgc.py's writes, which this sweep does not exercise. Expect these writes to be somewhat slower now that they are serialised. Negligible on the sweep's ~12s domains; worth measuring on a large production case.
The catalog already covers each singularity alone -- equator_straddle sits at lon -10, prime_meridian_seam at lat 45 -- but nothing covered the origin corner, where latitude and longitude change sign in the same domain. That is the case where a bug in one axis can be masked or amplified by the other, and it is cheap: a 6x6 degree box centred on 0N/0E. Tagged cheap so it runs in the default sweep, and both equator and seam so it is picked up by either tag slice.
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
1 participant
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.
Stacks on #265 (
refactor-obc-ic) — merge that first, it bringsREFERENCE_OCEAN.Descrip:
The suite only ever exercises
panama1— a 4°×3° box at 278°E/7°N, the easiest corner of the parameter space (northern, mid-latitude, 0–360, no seam, axis-aligned). Every seam and polar bug we've hit was found by hand on a one-off case, not by the suite. This adds a catalog of 17 domains and sweeps grid + bounding-box tests across all of them. Pure in-memory grids, no network, no CESM — the full sweep is 8 seconds.Changes:
tests/fixtures/domains.py— the whole framework in one file:DomainSpecdataclass, 17-row catalog, selection function, fixtures. Auto-discovered by the existingfixtures/*.pyglob, so adding a domain is adding one row. Covers seams (prime meridian, dateline), convention pairs (the same domain spelled 0–360 and −180–180), polar caps (EPSG:3995/3031), rotated, thin/tiny, and theRectilinearCartesianSupergridbranch nothing else exercises.tests/domains/test_grids.py— grid invariants across the catalog: builds, coordinates finite and on-sphere, supergrid shape,is_rectangular(), all seven metrics finite and positive,write_supergrid→from_supergridroundtrip.tests/domains/test_bounding_boxes.py— sweepsGrid.get_bounding_boxes, the chokepoint between a domain and the data downloaded for it. The span assertion now uses the largest angular gap between successive longitudes, distinguishing a genuinely global range (a polar cap) from a small domain that merely straddles the antimeridian.rotated_on_datelineis xfailed: 5.2° true span, 360° bbox.Selection flags:
--all-domains,--domain-tags=seam,polar,--domains=arctic_cap; default is a 6-domain cheap tier. A bad key or tag raisesUsageErrorat collection, listing the valid ones.recipe.build_griddispatches ongrid.type(mirroringbuild_topo/build_vgrid) sofrom_projectionandfrom_centerare reachable from YAML. They weren't before — a polar cap or rotated domain could not be expressed in a config at all, not from the CLI, not fromcrocodash create, not from MCP.Minor changes:
--runslow; it reimplemented-m "not slow". The ~14@pytest.mark.slowdecorations stay as plain markers, and CI's pytest line gains-m "not slow"to preserve current behaviour.environment.ymldeclarespyproj, needed byfrom_projection/from_centerand currently arriving only transitively.REFERENCE_OCEANsynthesizes data for whatever bbox it's handed, so a wrong bbox still yields clean output — the sweep passed for that reason, not because the boxes were right. Bounding-box correctness is asserted directly instead; the pipeline sweep goes to crocontainer#7.