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

Feature soil organic content #434

Merged
merged 9 commits into from
Oct 23, 2024
Merged

Feature soil organic content #434

merged 9 commits into from
Oct 23, 2024

Conversation

dschlaep
Copy link
Member

See #397

  • SOILWAT2 now represent the influence of soil organic matter on the soil water retention curve and the saturated hydraulic conductivity parameter

  • Saturated percolation is now limited. The upper bound is a function based on the saturated hydraulic conductivity parameter (which includes effects of organic matter), frozen soils, and a user-specified "permeability" factor.

  • New input via "siteparam.in" to specify the depth at which characteristics of organic matter have completely switched from fibric to sapric peat

  • New input via "soils.in" to provide the proportion of soil organic matter to bulk soil by weight for each soil layer.

  • New input via "swrc_params*.in" to provide parameter values of the soil water retention curve representing fibric and sapric peat.

- SWC-related variables need to be (re-)initialized after soils are updated
- tests for SWRCBulkSoilParameters()
- water balance tests with soil organic matter > 0
…maters

- for now, set om to 0 for water balance test
Close #397

The implemented approach has two steps:
1) Determine organic matter properties for the soil layers assuming fibric peat characteristics at the soil surface and characteristics of sapric peat at a user-specified depth (with a default of 50 cm).
2) Estimate bulk soil parameters of the soil water retention curve as linear combinations of properties for the mineral soil component and of properties for the organic matter soil component using the proportion of organic matter in the bulk soil as weights.

New inputs
* siteparam.in: depthSapric
* soils.in: fractionWeight_om
* swrc_params*.in: swrcpOM[fibric], swrcpOM[sapric]

New functions
* interpolateFibricSapric() to linearly interpolate organic matter parameter between fibric and sapric characteristics
* SWRC_bulkSoilParameters() to calculate parameters of the bulk soil are as the weighted average of parameters of the organic and mineral components

Functions with new arguments/behavior
* SW_SIT_read() now reads in value for depthSapric from "siteparam.in"
* SW_LYR_read() now reads in values for fractionWeight_om from "soils.in"
* SW_SWRC_read() now reads in values for swrcpOM[fibric] and swrcpOM[sapric] from "swrc_params*.in"
* SW_SIT_init_run() now runs checks for swrcpOM[fibric] and swrcpOM[sapric] and
calculates bulk soil SWRCp from organic and mineral soil components via a call to SWRC_bulkSoilParameters()
* SW_swcBulk_saturated() gained argument `"fom"` which is passed on to PTF_Saxton2006() if legacy `"Cosby1984AndOthers"` is selected as PTF
* SW_swcBulk_minimum() and ui_theta_min() gained argument `"fom"` which is passed on to PTF_RawlsBrakensiek1985() if legacy `"Cosby1984AndOthers"` is selected as PTF
* PTF_Saxton2006() gained argument `"fom"` and throws an error if
fom > 0.08 (these published equations incorporate influence of organic matter up to 8% which was previously fixed at 0%)
* PTF_RawlsBrakensiek1985() gained argument `"fom"` and throws an error if
fom > 0
* SW_check_soil_properties() now checks that user-input `fractionWeight_om` is within 0-1
* set_soillayers() gained argument `"pom"` to set a value for `fractionWeight_om`
** This changes simulation output **
* Different outcomes during periods of high infiltration (e.g., snowmelt) and during conditions of low hydraulic conductivity (e.g., frozen soils, sapric organic matter).

* Saturated percolation is now limited -- previously, it was unbounded. The upper bound is a function based on saturated hydraulic conductivity (which includes effects of organic matter), frozen soils, and a user-specified `"permeability"` factor.

* Bulk soil saturated hydraulic conductivity parameter now accounts for flow pathways through organic matter above a threshold and assumes conductivities through mineral and organic components in series outside of those pathways.
@dschlaep dschlaep linked an issue Oct 21, 2024 that may be closed by this pull request
@dschlaep dschlaep added this to the Release v8+ milestone Oct 21, 2024
@dschlaep dschlaep self-assigned this Oct 21, 2024
Copy link

codecov bot commented Oct 21, 2024

Codecov Report

Attention: Patch coverage is 83.19328% with 20 lines in your changes missing coverage. Please review.

Project coverage is 73.35%. Comparing base (2351511) to head (861950e).
Report is 10 commits behind head on release/devel_v8.1.0.

Files with missing lines Patch % Lines
src/SW_Site.c 81.48% 20 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##           release/devel_v8.1.0     #434      +/-   ##
========================================================
+ Coverage                 73.33%   73.35%   +0.01%     
========================================================
  Files                        20       20              
  Lines                      6233     6308      +75     
========================================================
+ Hits                       4571     4627      +56     
- Misses                     1662     1681      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- SWRC_bulkSoilParameters(): added documentation of argument "swrc_type" to fix documentation
- SWRC_bulkSoilParameters(): corrected spelling of argument "swrcOM" to "swrcpOM"
- tests "SWFlowSaturatedPercolation", "SWRCBulkSoilParameters" and "SitePTFRawlsBrakensiek1985": clang-tidy: use const variables where appropriate
- test "SWFlowSaturatedPercolation": clang-format
- the variable standingWater was previously used uninitialized which lead to NaN when compiled with g++
Previously, SWRC_bulkSoilParameters() was called with "SW_Site->swrcpOM" as argument for "const double swrcpOM[2][SWRC_PARAM_NMAX]" to pass fibric and sapric organic matter SWRC parameters.

Problems (here using gcc/g++ v14.2.0):
- gcc warning (`CC=gcc make clean all`): "'SWRC_bulkSoilParameters' accessing 96 bytes in a region of size 48 [-Wstringop-overflow=]"
- gcc error (`CC=gcc make clean bin_debug_severe`): "invalid use of pointers to arrays with different qualifiers in ISO C before C23 [-Werror=pedantic]"
- g++ error (`CXX=g++ make clean test`): "cannot convert 'const double (*)[]' to 'const double (*)[6]'"

Option 1: coerce call with "(const double (*)[SWRC_PARAM_NMAX]) SW_Site->swrcpOM" as argument for "const double swrcpOM[][SWRC_PARAM_NMAX]"
- gcc warning (`CC=gcc make clean all`) resolved
- gcc error (`CC=gcc make clean bin_debug_severe`) resolved
- g++ error (`CXX=g++ make clean test`) resolved

Option 2: remove "const", i.e., call with "SW_Site->swrcpOM" as argument for "double swrcpOM[][SWRC_PARAM_NMAX]"
- gcc warning (`CC=gcc make clean all`) resolved
- gcc error (`CC=gcc make clean bin_debug_severe`) resolved
- g++ error (`CXX=g++ make clean test`) resolved

This commit implements option 2.

Thanks to @N1ckP3rsl3y
@dschlaep dschlaep requested a review from N1ckP3rsl3y October 23, 2024 15:31
tests/gtests/test_SW_Flow_Lib.cc Outdated Show resolved Hide resolved
- commit 8029a9d introduced for the test SWFlowTest.SWFlowSaturatedPercolation a new variable "swc0init' to initialize soil water content
- by mistake, the first use of "swc" was, however, not initialized

-> this commit now initializes "swc[]" as intended
@N1ckP3rsl3y N1ckP3rsl3y self-requested a review October 23, 2024 18:11
@dschlaep dschlaep merged commit f254f40 into release/devel_v8.1.0 Oct 23, 2024
12 checks passed
@dschlaep dschlaep deleted the feature_soc branch October 23, 2024 18:16
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.

Organic soil content: influence on soil water retention
2 participants