Repository navigation
Fix stale 'ocnice' grid directory references in case creation - #45
Merged
Merged
Conversation
CrocoDash's Case creation writes MOM6/CICE/WW3 grid files into separate ocean/sea_ice/wave subdirectories, but several call sites here still looked for a single combined "ocnice" directory that no longer exists. This made _update_component_grids_xml's ocn_dir existence assertion fail unconditionally for every custom-grid case, which was silently swallowed by CrocoDash's Case.__init__ -- case.setup never ran, and MOM6's grid namelist section (NIGLOBAL, GRID_CONFIG, ...) never got written to user_nl_mom, only surfacing as a KeyError at build time. get_custom_ocn_grid_path() now points at "ocean"; new get_custom_ice_grid_path()/get_custom_wave_grid_path() cover CICE's grid file and WW3's *.inp files, which live in their own subdirectories.
Needed by CrocoDash's process_cice_forcing (extract_forcings/cice.py), which calls supergrid.expand(n_halo_cells) to grow the CICE grid by a halo before regridding -- the previously-pinned commit predates that method entirely. Points at NCAR/mom6_forge#122 (still open upstream; rebase onto its merge commit once that lands).
manishvenu
added a commit
to CROCODILE-CESM/CrocoDash
that referenced
this pull request
Aug 7, 2026
visualCaseGen previously looked for a single combined "ocnice" grid directory that no longer exists (CrocoDash's Case creation writes ocean/sea_ice/wave separately), which silently broke case.setup for every custom-grid case -- see ESMCI/visualCaseGen#45. Its embedded external/mom6_forge was also several months stale and missing SupergridBase.expand(), needed by process_cice_forcing.
Picks up NCAR/mom6_forge#112 (calendar-arg-chl), merged with the existing expand() bump. interpolate_and_fill_seawifs() now accepts a calendar kwarg, matching what CrocoDash's process_chl() has been passing since CrocoDash#220.
alperaltuntas
requested changes
Sep 10, 2026
Rename the custom grid subdirectories to match CIME component naming: ocean -> ocn, sea_ice -> ice, wave -> wav. Applied at every site, not just the accessors -- including the WW3 *.inp glob in case_creator, where a stale name silently copied zero files rather than erroring. Also create the CICE grid directory in _create_notebook_object. The generated notebook calls Topo.write_cice_grid, which (unlike write_ww3_input) does not create parent directories, so a CICE-active case previously failed with FileNotFoundError.
alperaltuntas
requested changes
Sep 11, 2026
alperaltuntas
left a comment
alperaltuntas
left a comment
Member
There was a problem hiding this comment.
Below are a couple more line comments. Also, the mom6_forge commit hash doesn't appear to be pointing to a commit that's in the main branch. Could you elaborate on that?
Collaborator
Author
|
Reverted the m6f change, not sure what that was there for. |
Collaborator
Author
|
Do you mind if we bring in mom6_forge main on this PR? |
alperaltuntas
approved these changes
Sep 16, 2026
manishvenu
added a commit
that referenced
this pull request
Sep 16, 2026
manishvenu
added a commit
to CROCODILE-CESM/CrocoDash
that referenced
this pull request
Sep 16, 2026
Bumps visualCaseGen to the crocodash branch, which now carries ESMCI/visualCaseGen#45: case creation looks for custom grid files under ocn/, ice/ and wav/ instead of a single ocnice/ directory. Moves CrocoDash to write them there: ocn/ MOM6 supergrid, topo, vgrid, SCRIP, ESMF mesh + all forcing output ice/ cice_grid_*.nc wav/ WW3 *.inp files The submodule bump and the source change are in one commit on purpose: if visualCaseGen looks for ocn/ before CrocoDash writes it (or vice versa), the ocn_dir.exists() assert in _update_component_grids_xml fails, is swallowed by a broad except, and only resurfaces as KeyError: 'NIGLOBAL' at case.build. cime.domains["ocnice"] in case.py is left alone -- that is a CIME component-grid key, unrelated to the directory layout. duplicate() now carries over ice/ and wav/ as well, since those files used to ride along inside the single ocnice/ directory.
alperaltuntas
pushed a commit
to CROCODILE-CESM/CrocoDash
that referenced
this pull request
Sep 18, 2026
* Split inputdir/ocnice into ocn/, ice/ and wav/ Bumps visualCaseGen to pick up ESMCI/visualCaseGen#45, which makes case creation look for custom grid files under ocn/, ice/ and wav/ instead of a single ocnice/ directory, and moves CrocoDash to write them there: ocn/ MOM6 supergrid, topo, vgrid, SCRIP, ESMF mesh + all forcing output ice/ cice_grid_*.nc wav/ WW3 *.inp files The submodule bump and the source change are in one commit on purpose: if visualCaseGen looks for ocn/ before CrocoDash writes it (or vice versa), the ocn_dir.exists() assert in _update_component_grids_xml fails, is swallowed by a broad except, and only resurfaces as KeyError: 'NIGLOBAL' at case.build. cime.domains["ocnice"] in case.py is left alone -- that is a CIME component-grid key, unrelated to the directory layout. duplicate() now carries over ice/ and wav/ as well, since those files used to ride along inside the single ocnice/ directory. * Split inputdir/ocnice into ocn/, ice/ and wav/ Bumps visualCaseGen to the crocodash branch, which now carries ESMCI/visualCaseGen#45: case creation looks for custom grid files under ocn/, ice/ and wav/ instead of a single ocnice/ directory. Moves CrocoDash to write them there: ocn/ MOM6 supergrid, topo, vgrid, SCRIP, ESMF mesh + all forcing output ice/ cice_grid_*.nc wav/ WW3 *.inp files The submodule bump and the source change are in one commit on purpose: if visualCaseGen looks for ocn/ before CrocoDash writes it (or vice versa), the ocn_dir.exists() assert in _update_component_grids_xml fails, is swallowed by a broad except, and only resurfaces as KeyError: 'NIGLOBAL' at case.build. cime.domains["ocnice"] in case.py is left alone -- that is a CIME component-grid key, unrelated to the directory layout. duplicate() now carries over ice/ and wav/ as well, since those files used to ride along inside the single ocnice/ directory.
alperaltuntas
added a commit
to CROCODILE-CESM/CrocoDash
that referenced
this pull request
Sep 18, 2026
* Fix MOM6/CESM output files stamping a calendar name FMS rejects 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. * Dedupe start_date/end_date and split step into GET/regrid chunk sizes 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. * Rename crocodash_state.json to _crocodash_state.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. * Generalize IC/OBC extraction into model-agnostic engines 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. * Add REFERENCE_OCEAN synthetic forcing product 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. * Merge forcing_configurations + extract_forcings into forcing/ Collapses the two parallel submodules (a registry of declarative BaseConfigurator classes, and free process_* functions dispatched by a hand-maintained if-ladder in driver.py) into one forcing/ package, one class per forcing type owning both its configure-time declaration and its process-time extraction logic. Adds BaseConfigurator.process_components, a {flag_name: method_name} manifest each class declares (empty if it has no process step; more than one entry for a one-to-many type like "conditions", which answers both "ic" and "bc"). driver.py's run_workflow dispatches generically by deserializing every configurator present in config.json and calling whichever of its process_components methods were requested -- no per-type branch to add by hand. cli.py's --flags and case.py's process_forcings() kwargs are now generated from the registry the same way, dropping the old process_bgc grouping (bgcic/bgcironforcing are now independent) and the ic/bc friendly-name special-casing. Introduces WorkflowContext (forcing/base.py) carrying the workflow-level paths (grid/topo/vgrid files, raw/regridded/output dirs) no single configurator owns but most process steps need, built once per run_workflow() call and passed into every process method. forcing/__init__.py auto-discovers every module in the package via pkgutil, matching the pattern raw_data_access/datasets already uses -- adding a new forcing type going forward is one new file with a BaseConfigurator subclass + process_components, nothing else to wire up. One documented exception to fully-generic dispatch: bgcrivernutrients depends on runoff's process step having already produced a mapping file, preserved as an explicit small ordering override in driver.py rather than a generic dependency system, since it's the only such case today. * Regenerate API docs for ic/mom6/reference modules * Fix CI: black formatting, stale check-glade exemption, regenerate API docs * Stop threading case_is_non_local through every XMLConfigParam configurator BaseConfigurator now exposes an is_non_local property sourced from the registry's live Case, and the base configure() applies it to any XMLConfigParam output automatically. A configurator that writes an XMLConfigParam no longer needs to declare/accept a case_is_non_local ctor arg just to reach apply() with the right --non-local behavior -- one fewer thing every future XML-writing configurator has to wire up by hand. * Make the BGCRiverNutrients->Runoff coupling declared and validated BGCRiverNutrientsConfigurator.process() used to reach directly into ctx.config["runoff"]["outputs"]["ROF2OCN_LIQ_RMAPNAME"] with no record of the dependency anywhere -- a rename on either side would only surface as a bare KeyError at process-time, and driver.py's ordering override list was hand-maintained separately from the actual coupling. - BaseConfigurator gains a declared `depends_on_outputs` manifest ({configurator_name: [output_names]}). - ForcingConfigRegistry.get_configurator_output() replaces the raw dict reach, with a clear error naming both configurators if the dependency isn't there. - driver.py derives its run-before ordering from depends_on_outputs instead of a separately hand-maintained override dict. - A new test statically confirms every declared dependency actually names a real configurator + output param, instead of only failing at process-time. - get_input_param_object/get_output_param_object now raise a dedicated UndeclaredParamError (KeyError subclass) instead of a bare KeyError, so a param-consistency failure can't be confused with an incidental KeyError from a configurator's own business logic. * Inline TidesConfigurator.tidal_data_str into mom6.py, gated by is_active ConditionsConfigurator was reaching into self.registry.active_configurators["tides"] and calling its tidal_data_str() method directly. tidal_data_str() is pure string formatting with no dependency on TidesConfigurator instance state, so it's inlined into mom6.py's own OBC-string building, gated only by self.registry.is_active("tides") -- matching the existing bgc branch right above it, which already only checks is_active("bgc") rather than reaching into the bgc configurator. Output is byte-identical; the now-unused tidal_data_str method is removed from tides.py. * Scope GLORYS RDA glob to the year subdirectory, not the whole archive get_glorys_data_from_rda did a fully recursive glob over the entire decades-spanning campaign storage tree once per requested day. Files are actually filed flatly under a year subdirectory matching each file's own date, so this walked the whole archive far more than needed -- multiplied across get_step_days-sized OBC chunks over a multi-decade date range, this made chunked OBC fetches impractically slow. * Add MARBL tracer support to REFERENCE_OCEAN synthetic product (#273) 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. * Add crocodash create --configure-only recipe.create_case_from_yaml has always taken a configure_only parameter -- build and configure a case but skip process_forcings, for callers that need to stage their own input data in between -- but the CLI never exposed it. It was exposed once, in 7e4d6f7 on the `cice` branch, whose only PR (#257) was closed in favour of the refactor-obc-ic stack. The commit is an ancestor of no live branch and of no open PR, so the flag exists nowhere in the repo today. It is not unused, though. crocontainer's run_regional_test.py invokes `crocodash create --configure-only` and its smoke test greps `crocodash create --help` for the flag. Those work only because crocontainer main still pins CrocoDash at exactly 7e4d6f7, so the published container image bakes in a CrocoDash carrying the flag. The next pointer bump on crocontainer main breaks both. Restoring it here, at the root of the open stack: this branch is the only open PR based directly on main, so one commit reaches main -- which is what the container image is built from, and therefore the only thing that actually unblocks crocontainer -- and propagates into all nine descendant branches without cherry-picking. cli.py is taken verbatim from 7e4d6f7. tests/test_cli.py is new: the CLI had no parser-level tests at all, and configure_only had no coverage on any branch. * Return the default from ask_yes_no on EOF, not False ask_yes_no() returned a hardcoded False when stdin was closed, ignoring its own `default` argument. Its sibling ask_string() already returned the default in that case, so the two prompts behaved inconsistently. The visible symptom is that ForkBundle.fork() could not run non-interactively at all: its final "Proceed with this configuration?" prompt is default=True, so an EOF turned into False and raised "Fork cancelled by user." The four _resolve_copy_plan prompts have the same problem in the other direction -- they rely on the implicit default=True, and silently copied nothing. Returning `default` matches ask_string and matches what pressing enter at an interactive prompt does, which is the sensible reading of a default. Recovered from 7e4d6f7 on the closed `cice` branch (#257), which is an ancestor of no live branch -- same orphaned commit that carried `crocodash create --configure-only`. Landing it here for the same reason: this is the only open PR based directly on main. * Serialise the dask-backed OBC writes to stop an intermittent deadlock 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. * Re-parent CESM output products onto MOM6ForcingProduct CESM_POP_OUTPUT and CESM_MOM_OUTPUT feed the MOM6 IC/OBC pipeline, but were left on plain ForcingProduct when write_metadata's include_marbl_tracers keyword moved down to MOM6ForcingProduct. They register fine and only fail at process_forcings time, with a TypeError on the keyword argument, making POP unusable as an IC/OBC product. Both classes already declare every attribute the MOM6ForcingProduct contract requires, so this is a pure re-parent. Adds regression tests pinning the inheritance, the exact write_metadata call the MOM6 forcing driver makes, and POP's marbl tracer merge. * Reject non-MOM6 forcing products in ConditionsConfigurator product_name validation only checked the generic ForcingProduct base, so a non-MOM6 forcing product (GLOFAS river discharge, a CICE restart, ...) passed as the conditions configurator's product_name slipped through and only failed later in configure() with a confusing "write_metadata() got an unexpected keyword argument 'include_marbl_tracers'" TypeError, since only MOM6ForcingProduct overrides write_metadata to accept that kwarg. Validate against MOM6ForcingProduct in the configurator, where every other per-model product check lives, splitting the unknown-product case out so the message says which of the two failed and pointing CICE products at cice_product_name. Case.configure_forcings keeps its own generic existence/ForcingProduct check unchanged. * Opt in to ignore_degenerate for the OBC regrid; bump rm6 Global forcing products have degenerate (duplicate/collapsed) cells near the poles -- GLORYS at its southernmost latitudes -- and ESMF aborts the whole regrid when it meets one: ESMC_FieldRegridStore failed with rc=506 (Degenerate Element Detected) so polar-domain OBC regrids fail outright. rm6 exposes ESMF's ignore_degenerate to skip those cells, defaulting it to False so a source grid malformed for some other reason still fails loudly; this passes True from the one call site that needs it. The rm6 bump (1c48b714 -> 74307b2) is what supplies the parameter -- the two have to move together, since the old pin would reject the keyword. 74307b2 is the tip of CROCODILE-CESM/regional-mom6#68, so this pin is interim: re-point it at rm6 main once that merges. The same parameter is proposed upstream in COSIMA/regional-mom6#325. Note the previous pin (rm6 main, 1c48b714) had no degenerate handling at all, while CrocoDash main pins 97dfb3a where it was hardcoded on -- so this restores to the stack a fix it had silently lost, now opt-in. * Repair the main-merge resolution of the OBC bathymetry bbox feature Merging main (PR #226, tmask-derived OBC download bboxes) into refactor-obc-ic dropped #226's changes into the pre-generalization body of process_obc_conditions, leaving three lines referencing names that no longer exist in that scope: - utils.build_forcing_request(product_info, function_args): product_info isn't a parameter of the generalized signature (it takes dataset_varnames), and this clobbered the variables/extra_args the caller had already resolved. - fill_method = rm6.regridding.fill_missing_data: rm6 isn't imported in obc.py anymore, and fill_method is set by mom6._regrid_obc_chunk. - the product_info["boundary_fill_method"] check, which is already done in mom6.process_mom6_obc. All three sit past the preview early-return, so any real OBC run would have raised NameError immediately after the bbox block. Drop them, and drop the now-dead function_args parameter -- callers resolve variables/extra_args themselves. The merge also added bathymetry_path to driver.py's call and to process_obc_conditions, but not to process_mom6_obc in between, so the driver failed with an unexpected-keyword TypeError before reaching any of the above. Thread it through both the preview and real branches. Finally, fix the test that was meant to pin this: it asserted on mock_mom6.process_obc_conditions, an attribute driver.py never calls, so call_args was None and the assertion raised AttributeError instead of checking that bathymetry_path reaches the OBC call. * Emit BGC tracer OBC params once, not once per boundary The OBC_DATA_<tracer> params sat inside the per-segment loop, so a four-boundary domain appended each of them to user_nl_mom four times. Unlike the physical tracers, which read a per-boundary forcing_obc_segment_<ix>.nc, each BGC tracer reads a single <tracer>_obc_segment.nc holding all four segments, so the param carries no segment index and belongs outside the loop. Also reconstruct these params in deserialize(), which only rebuilt the OBC_SEGMENT_* names and silently dropped the BGC ones on a round-trip. They are keyed by tracer rather than segment, so they are recovered by name prefix instead of being re-derived from the registry and product. * Split BGC tracers into per-tracer OBC files MOM6's generic tracer code reads BGC open-boundary data from one file per tracer holding every segment (<tracer>_obc_segment.nc), while the physical tracers are read per boundary from forcing_obc_segment_NNN.nc. The regrid step writes the BGC tracers into the per-boundary files alongside temp and salt, and the pipeline stopped at MERGE -- so the per-tracer files that OBC_DATA_<tracer> points at were never created. regional_mom6 already has reformat_bgc_tracers_into_files for exactly this, but its only caller is rm6's own setup_ocean_state_boundaries, which this pipeline does not go through. Do the split here instead, after MERGE. write_metadata merges marbl_var_names into tracer_var_names, which leaves the BGC tracers indistinguishable from temp/salt downstream; record the MARBL subset separately so the OBC step knows which tracers to split out. * Drop marbl_var_names from metadata when BGC tracers are not merged BaseProduct.write_metadata copies every public, JSON-compatible class attribute into the metadata dict, so marbl_var_names arrives in `base` before the include_marbl_tracers gate is ever consulted -- which made the `base["marbl_var_names"] = ...` assignment inside that gate a no-op. The consequence is a metadata dict that contradicts itself when BGC is off: marbl_var_names is populated, but tracer_var_names was never merged, so the BGC tracers are never regridded. process_mom6_obc then reads the presence of marbl_var_names as "these tracers were regridded" and _split_bgc_tracers_into_files dies on the first one: KeyError: "BGC tracer variable 'PO4_segment_001' not found in .../forcing_obc_segment_001.nc. Expected it there because 'PO4' was included in the regridded tracer set." Any non-MARBL case built on a product that merely declares marbl_var_names hits this -- CESM_POP_OUTPUT declares it so a %MARBL-BIO compset can use it, and process_forcings then fails for every other compset. Pop the key in the non-BGC branch so its presence means what the gate already intends it to mean: the BGC tracers really are in the regridded tracer set. * Address PR review: Calendar as the calendar contract, OBC/BGC and config fixes Calendar: - ForcingProduct must now declare `calendar` as a Calendar instance; the cf/cesm/mom6 names can no longer be declared individually and so cannot drift apart. - write_metadata expands dataclasses into named dicts, so the Calendar survives into config.json intact; Calendar.from_config reads it back. - Writers take the Calendar, not a string. bgc.process_river_nutrients uses .cf for its cftime calls and .mom6 for the attribute it writes; chl and convert_cftime_to_numeric likewise name the role they need. - Case.configure_forcings now xmlchanges CALENDAR, which previously had no consumer, so CESM no longer runs NO_LEAP against Gregorian forcing. OBC: - Open each per-boundary file once in the BGC split rather than once per tracer, and stream to_netcdf from the open sources. - Drop the BGC tracers from the per-boundary files after splitting; they are never read from there and were doubling the largest files on disk. Skip the split when the per-tracer files already exist so resume still works. - Remove the dead calendar/time_units kwargs block, which would have raised TypeError for duplicate keyword arguments had it ever fired. Other: - Raise with the date and pattern when the GLORYS RDA glob finds no file, instead of silently producing a dataset with a gap. - Pass default=False at the four _resolve_copy_plan prompts so a piped fork does not copy SourceMods or apply xmlchanges unconfirmed. - Tidy write_metadata's marbl_var_names handling. * Fix configurator smoke test and correct Calendar docs test_all_configurators_smoke builds ctor args by name and falls back to a dummy string, which the configurators then passed to dataclasses.asdict(). Give the typed `calendar` arg a real Calendar instead of loosening the configurators, which would reopen the drift the Calendar exists to prevent. Also correct two comments that conflated the two independent copies of the calendar in config.json: the product metadata block, written by write_metadata and read directly by the OBC step, and the chl / river-nutrients inputs block, which is what Calendar.from_config reads. * Only convert `calendar` to a dict when it is actually a Calendar BaseConfigurator.inspect() constructs a configurator with empty-string placeholders for every input param, and deserialize() rebuilds one from the stored dict, so the chl / river-nutrients constructors receive three shapes: a live Calendar, the serialized dict, or a placeholder. Converting unconditionally raised TypeError from asdict() on the latter two. * Guard the OBC bathymetry_path wiring; repair imports lost in the merge Adds test_process_bc_forwards_bathymetry_path, covering both the preview and real obc.process_obc_conditions calls. forcing/obc.py derives a per-boundary download bbox from the topography's ocean mask only when handed bathymetry_path, so without it every boundary silently downloads the full supergrid extent. The equivalent assertion on refactor-obc-ic could not be ported directly: this branch's driver dispatches through configurator process methods rather than process_mom6_obc. Also fixes three things the merge resolution dropped, all caught by the tests above: - tests/forcing/test_mom6_engine.py lost its `_split_bgc_tracers_into_files` import, so the three BGC split tests raised NameError rather than running. - tests/forcing/test_obc.py lost its `obc_module` alias. - test_chl / test_bgc construct their configurators directly, which now need a Calendar: the cf_calendar string and its silent "NOLEAP" fallback are gone, and defaulting there is the bug the Calendar exists to prevent. * Restore three behaviours lost in the forcing/ move Found by diffing the pre-move forcing_configurations/+extract_forcings/ tree against forcing/ function-by-function, since the rename makes the raw diff unreadable. Everything else compared clean. - _regrid_obc_chunk: restore ignore_degenerate=True. fd12b71 added it on refactor-obc-ic after this branch forked; every later merge applied it to extract_forcings/mom6.py, which no longer exists here, so it silently no-op'd. Without it polar domains abort the OBC regrid on ESMF rc=506 (Degenerate Element Detected). - driver: thread preview into WorkflowContext. _build_context never set it, so ctx.preview was always the False default and process_ic/ process_bc could not preview -- cli.py still passes preview through, so a preview request ran the real extraction. The dispatch loop's if/else on preview called the same thing in both arms; drop it, the process_* methods read ctx.preview themselves. - RunoffConfigurator.configure: size the smoothing params off case_esmf_mesh_path again, not rof_esmf_mesh_filepath. Slipped during the move; get_suggested_smoothing_params derives RMAX/FOLD from the average resolution of the *ocean* mesh. * Create IC's raw and output directories in ic.py process_initial_condition() wrote ic_unprocessed.nc into raw_data_dir without creating it. obc.py creates raw/regridded/output itself, so IC only succeeded when BC happened to run first -- on a fresh inputdir with IC ordered first it always failed. netCDF4 reports a missing parent directory as a bare EACCES, so the failure surfaced as PermissionError: [Errno 13] on the output file rather than anything pointing at the absent directory. mkdir after the preview early-return, so preview stays side-effect free. * Review Comments * Some Slow Test Fixes * Update to main rm6 * Back to rm6 main * Revert "Back to rm6 main" This reverts commit 2743a6d. * RM6 * Movement of chl.py * Bring in UptoDate VCG, m6f, rm6 for CI Tests * Up to date RM6 (without the newest chlorophyll * New rm6 main * Try This Fix * Using the rm6 final cholorphyll branch and visualCaseGen main * Use a version of visualcasegen that has mom6_forge nwest * Small Merge Failure * Update rm6 import * Added 3 tests to add coverage (ic is not covered at all and some obc stuff) * Drop preview assertions from the new IC/OBC coverage tests preview only reports filenames back to configure_forcings, so pinning it adds little over the tests that exercise the real path. * Pin serial ESMF in CI to stop MPI_Init aborting setup_case.py pytest itself passes; the job died two steps later, in "Run setup_case.py", the moment the topo regrid made its first xESMF call: ib_iface.c:1269 UCX ERROR mana_0: iface failed to create UD QP Abort(406972943) on node 0: Fatal error in internal_Init_thread ##[error]Process completed with exit code 15. environment.yml pins only esmpy, which resolves esmf to an mpi_mpich_* build. That build calls MPI_Init on first use, and on GitHub runners UCX cannot open the ib_verbs interface, so MPI aborts and takes the step with it. CrocoDash only uses ESMF serially, via xesmf, so the MPI stack buys nothing here -- pin the nompi_* build instead. The pin alone would not have taken effect: this workflow sets cache-environment with an explicit cache-environment-key, and create-args is not part of that key, so the cached env built against the mpi_* build would have been restored over it. Tag the key "nompi" so it busts once. Same root cause and fix as NCAR/mom6_forge#107, which saw it as exit 143 rather than 15. * Raise on unrecognized process_* kwargs in process_forcings Previously, an unrecognized process_* kwarg (a stale pre-refactor name like process_initial_condition, or a typo like process_ics) silently landed in **kwargs and was never read, so the real flag defaulted to True instead of being skipped as the caller intended. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Use graphlib.TopologicalSorter for stable, transitive process ordering The previous auto-enable/ordering logic in run_workflow had two bugs: it only pulled in a flag's direct dependencies (not transitive ones, so a 3-level chain like chaina -> chainb -> chainc would drop chainc unless it happened to be enabled some other way), and it built the final run order by iterating over `requested`, a set, so the order wasn't stable across PYTHONHASHSEED values. order_overrides is already {node: predecessors}, exactly the shape graphlib.TopologicalSorter expects, so hand it there directly for a deterministic order and CycleError detection instead of a hand-rolled loop that couldn't catch cycles. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add DATM Streams to fix two issues for the workshop (#278) * Align DATM/DROF stream years with the run dates and narrow the file list CDEPS hardcodes year_align=1 and the full year span for every JRA stream in stream_definition_{datm,drof}.xml, so a JRA case silently reads an arbitrary data year. The DATM_YR_ALIGN/_START/_END xml variables cannot fix it: only the CLM/GSWP3/NEON/CPLHIST stream sets reference them, and no compset pattern in datm's config_component.xml matches DATM%JRA, so they sit unread at 1/2004/2004. The only lever CDEPS offers is user_nl_<comp>_streams. Narrowing the file list matters as much as the alignment. <datafiles> is expanded from the stream definition's own year tags before user mods are merged, so retagging the years alone still enumerates all 59 JRA files, each of which CDEPS opens to build its time axis -- on GLADE that init can exceed the model run time. StreamYearConfigurator writes both, for DATM and DROF, from the dates already passed to configure_forcings. It sets year_align == year_first, which makes the data year equal the model year for every year in range. Streams whose coverage spans a single year (CORE2_NYF, the CORE_RYF*_JRA repeat years) are left alone, so climatology compsets are unaffected with no allowlist to maintain; a run falling outside a stream's coverage is reported and left at CESM defaults rather than failing the case. cdeps_streams.py reads through CIME's own parsers (StreamCDEPS, NamelistDefinition) rather than parsing the XML itself. That is what makes the mode-to-stream-name matching and the per-year datestamp resolution correct: JRA v1.5 splits 1958-2023 across four <file> entries, so 2020/2021/2022 resolve to three different datestamps. Those are private CDEPS methods, so the tests pin their output to exact filenames. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop lru_cache from cdeps_streams CIME's GenericXML already caches parsed trees, so memoizing _streams and stream_names here bought nothing measurable: 50 uncached _streams builds take 20ms, and a full DATM+DROF plan (stream_names + coverage + datafiles across all 8 CORE_IAF_JRA streams and rof.iaf_jra) runs in 0.146s, nearly all of it the one-time import and first parse. Removing it also drops the hashable-argument requirement the decorator imposed on cesmroot. * Fix check-glade and docs CI failures check-glade: tests/forcing/test_atm.py hardcodes the GLADE inputdata root as a stand-in DIN_LOC_ROOT for resolving CDEPS stream paths. The test is already gated on GLADE, so exempt it the way tests/forcing/test_bgc.py is. docs: regenerate the API docs for the two modules this branch adds, CrocoDash.forcing.atm and CrocoDash.forcing.cdeps_streams, using the command the workflow checks against: cd docs && sphinx-apidoc -o source/api-docs -H 'CrocoDash API Docs' \ --force ../CrocoDash ../CrocoDash/rm6 ../CrocoDash/visualCaseGen --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * Split inputdir/ocnice into ocn/, ice/ and wav/ (#280) * Split inputdir/ocnice into ocn/, ice/ and wav/ Bumps visualCaseGen to pick up ESMCI/visualCaseGen#45, which makes case creation look for custom grid files under ocn/, ice/ and wav/ instead of a single ocnice/ directory, and moves CrocoDash to write them there: ocn/ MOM6 supergrid, topo, vgrid, SCRIP, ESMF mesh + all forcing output ice/ cice_grid_*.nc wav/ WW3 *.inp files The submodule bump and the source change are in one commit on purpose: if visualCaseGen looks for ocn/ before CrocoDash writes it (or vice versa), the ocn_dir.exists() assert in _update_component_grids_xml fails, is swallowed by a broad except, and only resurfaces as KeyError: 'NIGLOBAL' at case.build. cime.domains["ocnice"] in case.py is left alone -- that is a CIME component-grid key, unrelated to the directory layout. duplicate() now carries over ice/ and wav/ as well, since those files used to ride along inside the single ocnice/ directory. * Split inputdir/ocnice into ocn/, ice/ and wav/ Bumps visualCaseGen to the crocodash branch, which now carries ESMCI/visualCaseGen#45: case creation looks for custom grid files under ocn/, ice/ and wav/ instead of a single ocnice/ directory. Moves CrocoDash to write them there: ocn/ MOM6 supergrid, topo, vgrid, SCRIP, ESMF mesh + all forcing output ice/ cice_grid_*.nc wav/ WW3 *.inp files The submodule bump and the source change are in one commit on purpose: if visualCaseGen looks for ocn/ before CrocoDash writes it (or vice versa), the ocn_dir.exists() assert in _update_component_grids_xml fails, is swallowed by a broad except, and only resurfaces as KeyError: 'NIGLOBAL' at case.build. cime.domains["ocnice"] in case.py is left alone -- that is a CIME component-grid key, unrelated to the directory layout. duplicate() now carries over ice/ and wav/ as well, since those files used to ride along inside the single ocnice/ directory. * fix chain test fixtures to declare the outputs they promise The dummy chain configurators added for the transitive-ordering test set depends_on_outputs to {"dummychainb": ["out"]} and {"dummychainc": ["out"]} but left output_params empty, so test_depends_on_outputs_targets_exist failed: the dependency named an output that the depended-on class did not declare. Declaring it in turn makes deserialize() require the value, so the serialized stubs in the test config carry it too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: alperaltuntas <alperaltuntas@gmail.com>
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
2 participants
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.
Summary
Casecreation writes MOM6/CICE/WW3 grid files into separateocean/sea_ice/wavesubdirectories, but several call sites here still looked for a single combinedocnicedirectory that no longer exists on disk._update_component_grids_xml'socn_direxistence assertion fail unconditionally for every custom-grid case created throughCaseCreator(add_grids_to_ccs_config=Falsepath). TheAssertionErrorwas silently swallowed by the caller's broadexcept Exception, socase.setupnever actually ran and the MOM6 grid namelist section (NIGLOBAL,GRID_CONFIG,GRID_FILE, ...) never got written touser_nl_mom-- the case looked like it was created successfully, but only surfaced as aKeyError: 'NIGLOBAL'much later, atcase.buildtime.get_custom_ocn_grid_path()now points at"ocean"(matching where MOM6's supergrid/topo/vgrid/scrip/ESMF-mesh files actually live). Added two new sibling methods,get_custom_ice_grid_path()(->"sea_ice") andget_custom_wave_grid_path()(->"wave"), since CICE's grid file and WW3's.inpfiles live in their own subdirectories, not alongside the ocean grid files.Test plan