-
Notifications
You must be signed in to change notification settings - Fork 22
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
Water demand and allocation in sbm
model concept
#402
Conversation
as part of vertical SBM concept
for each sector a struct is defined
- all structs part of SBM (domestic, industry, livestock, nonpaddy and paddy) - for now assumption is that water demand arrays have the same lenght as SBM (can change later)
- soil water pressure heads of the root water uptake (rwu) reduction function (Feddes) can be provided (netCDF) - potential transpiration is partioned over depth using the root fraction - h1 of the rwu reduction function can be provided, either as 1.0 (rwu not constrained during saturated conditions) or 0.0 (rwu constrained during saturated conditions)
was missing in previous commit
- update unit tests - limit nonpaddy irrigation gross demand by soil infiltration capacity - include irrigation efficieny in gross demand calculation
now after snow and before recharge update
total_gross_demand, fraction groundwater used (irrigation and non irrigation) and allocation areas.
and added field `volume` to lateral kinematic wave subsurface flow
to kinematic wave for river flow, this is computed actual surface water abstraction from `WaterAllocation`. A check is maded for external negative inflow (local abstraction), and local available surface water volume for surface water abstraction is adjusted.
- set surface water `abstraction` from update of `WaterAllocation` - fix update of `act_surfacewater_abst` at allocation area level (should be added to local abstraction).
Errors related to variable names and indexing.
actual groundwater abstraction from `WaterAllocation` is added to `recharge` of the model type `sbm`.
- use one value for fraction surface water (no subdivision between irrigated and non-irrigated demands) - livestock can abstract from both surface and groundwater (was limited to surface water)
- groundwater demand calculation not correct (units) - reset `surfacewater_alloc` (0.0) for all cells
- add allocated irrigation water to SBM - compute return flow for non irrigation water withdrawal - add units/descriptions to fields of structs water demand
- split waterallocation between river and land domain, this is also easier for writing output - add non irrigation returnflow to river and overland flow kinematic wave routing
- fix check to apply `irri_alloc` in `SBM` - update waterdemand settings (read from TOML)
Use root fraction (length) for each unsaturated zone layer to scale required irrigation demand (only root zone). Fix computation infiltration_capacity and irri_dem_gross for nonpaddy irrigation (removed from unsaturated zone layers loop).
exclude lake and reservoir areas
Model parameter `rootfraction` and partitioning of potential transpiration over soil layers.
src/flow.jl
Outdated
@@ -29,6 +30,7 @@ abstract type SurfaceFlow end | |||
lake_index::Vector{Int} | "-" # map cell to 0 (no lake) or i (pick lake i in lake field) | |||
reservoir::R | "-" | 0 # Reservoir model struct of arrays | |||
lake::L | "-" | 0 # Lake model struct of arrays | |||
waterallocation::W | "-" | 0 # Water allocation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just allocation
? It's also abstraction
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with replacing waterallocation
by allocation
.
src/utils.jl
Outdated
function divide(x, y; max = 1.0, default = 0.0) | ||
z = y > 0.0 ? min(x / y, max) : default | ||
return z | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it is more explicit to name this something like bounded_divide
. Would be good to add a docstring and unit tests as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, changed the function name and added a docstring and unit tests as well.
fill(0.1, ncell), # specific storage | ||
fill(1.0, ncell), # storativity | ||
fill(0.0, connectivity.nconnection), # conductance | ||
fill(0.0, ncell), # total volume that can be released | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like something like Deltares/Ribasim#1566 may be useful for you as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for most structs we already make use of this, probably good to extend this to smaller structs as well as part of the v1
release.
test/vertical_process.jl
Outdated
) | ||
head = Wflow.head_brooks_corey(0.25, 0.6, 0.15, 10.5, -10.0) | ||
@test head ≈ -90.6299820833844 | ||
h3 = Wflow.feddes_h3(-300.0, -600.0, 3.5, Second(86400)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like these functions are a bit undertested. The functions have several if branches, but we test only one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, tests are extended.
@@ -61,6 +61,13 @@ profile `kv` is used and `z_layered` is required as input. | |||
| **`waterfrac`** | fraction of open water (excluding rivers) | - | 0.0 | | |||
| **`pathfrac`** | fraction of compacted area | - | 0.01 | | |||
| **`rootingdepth`** | rooting depth | mm | 750.0 | | |||
| **`rootfraction`** | root fraction per soil layer (relative to the total root length) | - | - | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the total root length here the length from the surface to the bottom of the longest root? Or is it the total length of all roots, such that it can take into account that some depths have a higher root density?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can indeed take into account that some depths have a higher root density, changed the description to make this more clear.
# correct rooting depth for soilthickness | ||
rootingdepth = @. min(0.99 * soilthickness, rootingdepth) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also consider throwing an error. Perhaps more annoying at first, but also less magical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I agree with an error (due to different underlying datasets of soilgrids and the LULC mapping tables of hydromt_wflow). But perhaps showing a warning message indicating (in how many cells) this correction occurs would be nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the preferred approach is to catch and correct this during model building as a pre-processing step. By the way, this correction or magical step was already implemented in the wflow Python version.
src/sbm.jl
Outdated
availcap = | ||
min(1.0, max(0.0, (sbm.rootingdepth[i] - sbm.sumlayers[i][k]) / usl[k])) | ||
end | ||
maxextr = usld[k] * availcap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tabs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed tabs.
src/sbm.jl
Outdated
end | ||
maxextr = usld[k] * availcap | ||
# the rootfraction is valid for the root length in a soil layer, if zi decreases the root length | ||
# the rootfraction needs to be adapted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
src/sbm.jl
Outdated
|
||
Update water demand for vertical `SBM` concept for a single timestep. Water demand is | ||
computed for sectors `industry`, `domestic` and `livestock`, and `paddy` rice fields and | ||
`nonpaddy` (other crop) fields. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`nonpaddy` (other crop) fields. | |
`nonpaddy` (other crop) fields. |
Might be good to look into pre-commit to remove trailing whitespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
@unpack subdomain_order, topo_subdomain, indices_subdomain, upstream_nodes, area = | ||
network |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated, but I'd replace all unpack macros by the now built in destructuring, (; a, b) = c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good suggestion, definitely something for v1
work!
Before updating surface water and groundwater allocations and abstractions reset to zero, so it is independent of the update at local and allocation area levels.
Water bodies were excluded in the computation of `groundwater_demand`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice work! A couple of comments on top of @visr's comments.
# correct rooting depth for soilthickness | ||
rootingdepth = @. min(0.99 * soilthickness, rootingdepth) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I agree with an error (due to different underlying datasets of soilgrids and the LULC mapping tables of hydromt_wflow). But perhaps showing a warning message indicating (in how many cells) this correction occurs would be nice
Unit of this variable is mm/t so `maximum_irrigation_rate` is a better variable name. Also fixed a couple of typos in docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't really have time for another review round, but saw that my comments were being addressed, so for me this is good to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing all comments (also quickly looked at the responses to Martijn's comments)! Looks good to me!
Issue addressed
Fixes #223 and fixes #102
Explanation
Support water demand and allocation in the
sbm
model concept. Water demand (gross and net) for the sectors industry, domestic and livestock can be provided (optional) to the wflow_sbm model as input. Irrigation demand for paddy areas (Paddy
) or other crops (NonPaddy
), both optional, is computed during the simulation. Water is first abstracted from surface water, first locally (grid cell) and then for allocation areas. For the remaining water demand groundwater (first locally and then for allocation areas) is abstracted. Return flows are computed for water demand sectors industry, domestic and livestock.Checklist
master
Rendered docs in e.g. https://deltares.github.io/Wflow.jl/previews/PR402/model_docs/vertical/sbm/#sbm_demand_allocation