Skip to content

For glacier initial conditions - #167

Merged
DarriEy merged 5 commits into
symfluence-org:developfrom
ashleymedin:develop
Jun 9, 2026
Merged

DarriEy merged 5 commits into
symfluence-org:developfrom
ashleymedin:develop

Conversation

@ashleymedin

Copy link
Copy Markdown
Contributor

This is some fixes fro the the glacier initial conditions. We still need to process the debris and bottom topography data to get the actual initial conditions.

* commit '05ca673cc940056a6f9a5c55e45b1c2c587987cd': (86 commits)
  fix(ci): follow symlink in arm64 binary arch verification
  fix(ci): green up Windows unit tests and arm64 dep install
  ci(deps): guard install-manifest consistency; fix cdsapi drift (symfluence-org#156 / review item 16)
  ci(release): build linux-x86_64 in an old-glibc container (symfluence-org#156 G6)
  fix(release): bundle the full recursive lib closure on Linux (symfluence-org#156 G6)
  refactor(install): tighten G7 workspace search + unify data-dir resolution (symfluence-org#156)
  fix(npm): glibc baseline must scan bundled libs; rocky dnf curl conflict (symfluence-org#156 G6)
  refactor(npm): single npm-bundle locator + drop models->cli import (symfluence-org#156 G6)
  fix(mesh): report output-coverage shortfall instead of opaque failure
  ci(npm): multi-distro validation of the released binary tarball (symfluence-org#156 G6)
  fix(npm): preflight tooling + post-extract soname/glibc self-check (symfluence-org#156 G6)
  fix(npm): resolve npm-bundled binaries from the model runner (symfluence-org#156 G6)
  ci(install): aarch64 source-build verification workflow (symfluence-org#156 G1)
  fix(install): deterministic binary-install location with cwd inference (symfluence-org#156 G7)
  fix(paths): resolve nested catchment/basin shapefiles across non-handler sites
  fix(observations): share nested-catchment resolver across all handlers
  fix(observations): dedup WSC download and resolve nested GRACE catchment
  fix(models): alias CLM-ParFlow->CLMPARFLOW; surface meshflow exceptions
  fix(clm): auto-resolve CESM inputdata under the data dir by default
  fix(models): resolve model-ensemble run failures (HEC-HMS, LSTM/GNN, CRHM, HYPE, CLM)
  ...
@DarriEy
DarriEy changed the base branch from main to develop June 8, 2026 23:04
Clear the ruff errors blocking CI on the glacier-initial-conditions work:
auto-fixable trailing whitespace / unused `datetime` import / import sort,
remove dead locals `num_gru` and `debris_thick_shp` (the latter unused in
has_glacier_data; process_glacier_attributes has its own), and comment the
`tsl_stats` computation to match its already-commented use. No behavior change.

Note: two pre-existing mypy signature/call mismatches remain (see PR comment) —
they were masked while ruff was failing and surface now that ruff passes.

Assisted-by: Claude (Anthropic)
@DarriEy

DarriEy commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Pushed a small commit fixing the ruff lint errors (trailing whitespace, an unused datetime import, import sort, and a few dead locals) so the linter passes and the PR now targets develop (the diff drops from 590 files vs main to 7 files vs develop).

With ruff green, CI's mypy step now runs (it was skipped before because ruff failed first) and surfaces 2 pre-existing type errors in src/symfluence/models/summa/glacier_manager.py. These are signature/call mismatches — flagging them for you since it's your glacier logic:

1. _create_attributes_glac_from_rasters — dead nGrid param
The signature takes nGrid: np.ndarray, but the body immediately overwrites it:

nGrid = grid_info['nGrid']

The call site (~L380) passes grid_info but not nGrid. Simplest fix: drop the nGrid parameter from the signature (it's redundant with grid_info['nGrid']).

2. _process_from_shapefiles call (~L168) — arg order
The call passes args in a different order than the signature:

  • dem_domain_shp / debris_thick_shp are swapped (the body reads dem_domain_shp→elevation and debris_thick_shp→debris, so the signature names are correct and the call is the one out of order)
  • glacier_dir is missing from the call (it's used in the body at L180/L184, and it is in scope at the call site)

Suggested call:

return self._process_from_shapefiles(
    domain_type_shp, debris_thick_shp, dem_domain_shp, glacier_dir,
    settings_dir, base_attributes_file, base_coldstate_file,
    hru_ids, gru_ids, hru_areas, hru_elevs, hru2gru
)

Happy to push these two fixes too if you'd like — just say the word. Left them for you since #2 changes which raster is read as dem vs debris.

Repo and code assistance from Claude (Anthropic).

@ashleymedin

Copy link
Copy Markdown
Contributor Author

Thanks @DarriEy please push those fixes.

DarriEy added 2 commits June 9, 2026 09:06
- _process_from_shapefiles call: reorder dem/debris shp args to match
  signature and pass the missing glacier_dir argument
- _create_attributes_glac_from_rasters: drop redundant nGrid parameter
  (the body overwrites it immediately from grid_info['nGrid'])

Fixes the 2 mypy [call-arg] errors surfaced in CI lint once ruff passed.

Assisted-by: Claude (Anthropic)
This PR removes the SETTINGS_SUMMA_INIT_GRID_FILE and
SETTINGS_SUMMA_ATTRIB_GRID_FILE config keys (schema fields, aliases, and
config_manager reads) as part of reworking glacier initial conditions, but
left the keys in three shipped example configs and the comprehensive
template. The strict shipped-config validation (RTI Q3) that landed on
develop after this branch's last CI run now rejects them as unknown keys.

Remove the now-dead keys from:
- examples/02_watershed_modelling/configs/config_wolverine_glacier.yaml
- examples/04_workshop_notebooks/config_logan_river_summa_asyncdds.yaml
- examples/04_workshop_notebooks/config_logan_river_summa_distributed.yaml
- resources/config_templates/config_template_comprehensive.yaml

Assisted-by: Claude (Anthropic)
@DarriEy
DarriEy merged commit 451cdc2 into symfluence-org:develop Jun 9, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants