Modular configs - #245
Open
JoshCu wants to merge 15 commits into
Open
Conversation
* add logic rules for constructing modular realizations * fix typos * add validation function * make sure all models in the list get the dependency check, allow sloth to be only model * add modular realization validation logic tests * add CI on feature branch * light reformat * Realization implementation (#232) * add realization template * add modular realization implementation * add basic templates for sloth/cfe/nom * fix typo, fix datetime parsing * add realization generation tests * reformat * add this branch to trigger ci * add branch to ci * Config implementation (#233) * add config generation function * remove CFE groundwater feature * add tests for modular config generation * add this branch to ci * light reformat * add logic and formatting edits Co-authored-by: Parth Modi <pamodi@ua.edu> * finalize rules Co-authored-by: Parth Modi <pamodi@ua.edu> * fix sac-sma typo * update tests * reformat * fix sac-sma typo --------- Co-authored-by: Parth Modi <pamodi@ua.edu>
* add logic rules for constructing modular realizations * fix typos * add validation function * add realization template * add modular realization implementation * add basic templates for sloth/cfe/nom * make sure all models in the list get the dependency check, allow sloth to be only model * add modular realization validation logic tests * add CI on feature branch * light reformat * fix typo, fix datetime parsing * add realization generation tests * reformat * add config generation function * remove CFE groundwater feature * add tests for modular config generation * add casam bmi config template * reformat casam config * config generation function * Realization implementation (#232) * add realization template * add modular realization implementation * add basic templates for sloth/cfe/nom * fix typo, fix datetime parsing * add realization generation tests * reformat * add this branch to trigger ci * add branch to ci * Config implementation (#233) * add config generation function * remove CFE groundwater feature * add tests for modular config generation * add this branch to ci * light reformat * add logic and formatting edits Co-authored-by: Parth Modi <pamodi@ua.edu>" * fix typos * add tooling to generate config and realization for casam * add casam realization tests * fix and update tests * fix comment * fix typo, set up test * update config test files * ruff format * remove stale imports * remove stale test function * Other realizations (#238) * add already developed models to modular realization implementation * add model append functionality * standardize realizations * update test files * fix tests * ruff format * removed redundant tests * add this pr to ci * ruff format * simply model appending * fix bug where ngen fails when sloth's model_params is an empty dict (#240) * fix bug where ngen fails when sloth's model_params is an empty dict * tests * ruff format * readd deleted test
* add CLI interface for modular coupling * simply model appending * clarify behavior * add tests * update readme with CLI usage details * remove unused import * Make modular configs properly setup output folders --------- Co-authored-by: chp2001 <chp.2001.jhp@gmail.com>
* added SUMMA config generation to modular config fn * tests * Modular refactor (#244) * remove duplicate code * handle calibrated parameters * remove model type name field, not used in ngen * reorganize functions * refactor tests * remove redundant tests * remove unused model cli args * remove awkward prompt placement * document calibrated parameter fetching * aggregate new model info into one place * remove hardcoded variable maps from templates * update stale comments * clarifying comments * pulled out awkward rule * remove confusing variable * add this branch to ci
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This was referenced Jul 24, 2026
Two developer-scaffolding changes crept into ci.yml on modular-configs and weaken CI: - pull_request.branches was widened to include modular-configs and summa-addition; revert to [main] so CI only gates PRs into main. - The unit-test job gained continue-on-error: true, which silently defeats the point of the new golden-test suite since a failing test can no longer fail the CI check. Remove it. The integration job's pre-existing continue-on-error: true (intentional, since it hits live S3/AORC data) is left untouched.
) The default (no --models) realization path in both the CLI and the map-app's /realization endpoint called create_modular_realization() but never called the sibling create_modular_configs(), so the resulting realization.json referenced cat_config/*.ini and troute.yaml files that were never written -- ngen would fail on the most common CLI invocation (cli -i cat-X -sfr, no --models). - ngiab_data_cli/__main__.py: default realization branch now also calls create_modular_configs(models=["sloth","nom","cfe"], routing=True), and hardcodes routing=True (default realizations always include t-route per the README, regardless of --routing). - map_app/views.py: get_realization() now calls create_modular_configs alongside create_modular_realization. - ngiab_data_cli/__main__.py: validate_input() no longer blocks on Prompt.ask when sys.stdin is not a TTY (piped input/CI/cron); it logs and proceeds as if the user answered "y". - Cleaned up pre-existing ruff findings (bare except, unused imports, formatting) in the two touched files so CI's whole-file lint pass. - tests/test_cli_args.py: added a test that validate_input does not invoke Prompt.ask when stdin is not a TTY, even with warnings.
…ry (#250) _handle_calibrated_params downloaded a calibrated realization template but never wrote config/realization.json -- the caller short-circuited on True right after the download, silently producing no realization for gages with calibrated CFE/NOM parameters. Now the downloaded template's time.start_time and time.end_time are patched with the caller's start/end times and the result is written straight to config/realization.json (replacing the dead-end downloaded_params.json write). Also dedupes two near-identical blocks in the same file: - MODEL_DEPENDENCY_RULES' four standalone-model tuples (lstm, lstm_rust, dhbv2, dhbv2_daily) collapse into a label map + shared predicate, spliced into the tuple via a generator expression. Message strings are unchanged. - dhbv2 and dhbv2_daily's identical 9-entry variables_names_map now share one module-level dict constant, since ModelSpec is frozen and callers always deepcopy before mutating. Adds test coverage for the calibrated-params path (success, failed download fallback, non-matching model combo, no gage_id), previously untested. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
The old per-model flags (--lstm, --lstm_rust, --dhbv2, --dhbv2_daily, --summa, --snow17, --sacsma) were removed from arguments.py in favor of a unified --models/--routing interface, but the README still documented them as live options and one example still used --lstm. Also corrects the Custom realizations section, which claimed SUMMA wasn't supported by --models even though MODEL_REGISTRY["summa"] and create_modular_configs() both support it (with the caveat that forcings must already exist since SUMMA config derives HRU order from them). Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
_configure_troute interpolated the raw output_folder/cat_id string directly into the sqlite gpkg lookup path and into troute.yaml's geo_file_path. FilePaths.geopackage_path always resolves to the bare leaf name, so any output_folder containing a path separator (e.g. -o some/nested/name) caused the gpkg lookup to miss and the generated troute.yaml to point at a nonexistent file. This restores the Path(cat_id).name normalization from 4c0a96e (#228), which was silently dropped when config generation was split out of create_realization.py into create_configs.py. Also drops a docstring reference to create_summa_realization, which no longer exists. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
The most recent commits (by me) resolve most linting errors (doesn't change functionality or documentation), and also gets rid of the unused CLI args (eval and vis) |
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
3 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.
bundles up several changes
#243 #242 #231 #237