Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration file and parameter read-in changes #48

Merged
merged 8 commits into from
Feb 28, 2022

Conversation

madMatchstick
Copy link
Contributor

@madMatchstick madMatchstick commented Feb 22, 2022

There are 4 main (mostly minor) config/param changes here:

  1. Soil parameters exponent_primary and exponent_secondary are now optional params read-in from *_config.txt; defaults to 1.0 otherwise.
  2. Both gw_storage and soil_storage no longer allow option to read-in as percentage %, but only literal value.
  3. Adjusted all parameters in *_config.txt to match ranges from NWM
  4. Adding [units] to *_config.txt

Additions

Changes

  1. Soils exponent_primary and exponent_secondary read-in via read_init_config_cfe(). Otherwise, set default value to 1.0
  2. Parameters gw_storage and soil_storage must only be literal values in config
  3. Cgw and max_gw_storage values were significantly too large, adjusted to comply with NWM ranges for all (7) cat_*_bmi_config*.txt.
  4. read_init_config_cfe() checks for [units]
    • e.g. soil_params.depth=2.0[m] or if unitless soil_params.b=4.05[] or similarly, just soil_params.b=4.05
    • Only for unit-worthy params, initial read-in checks for existence of [units] in config, and prints warning if missing. E.g. see soil_params.satdk

Removals

Testing

  1. make_and_run_bmi.sh results the same before and after changes.
  2. BMI unit tests pass

Todos

  • Remove // old code commented out once PR approved
  • Update user docs/readmes/install for parameter adjustments

@lcunha0118
Copy link
Contributor

I tested and it all look good. For future reference, we might want to include warnings when the parameter values are unrealistic (negative).

@SnowHydrology SnowHydrology merged commit e1ea4a1 into NOAA-OWP:master Feb 28, 2022
@madMatchstick madMatchstick mentioned this pull request Mar 1, 2022
11 tasks
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.

4 participants