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

Bugfix subbasin #263

Merged
merged 7 commits into from May 7, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog.rst
Expand Up @@ -25,6 +25,7 @@ Fixed
-----
- Wrong dtype for wflow_subcatch map. PR #247,
- Removed building a wflow model without a config file in the build notebook.
- Deprecated np.bool and earlier error message for subbasin delination. PR #263

v0.5.0 (February 2024)
======================
Expand Down
7 changes: 7 additions & 0 deletions hydromt_wflow/wflow.py
Expand Up @@ -417,6 +417,13 @@ def setup_rivers(
"""
self.logger.info("Preparing river maps.")

# Check that river_upa threshold is bigger than the maximum uparea in the grid
hboisgon marked this conversation as resolved.
Show resolved Hide resolved
if river_upa > self.grid[self._MAPS["uparea"]].max():
raise ValueError(
f"river_upa threshold {river_upa} should be larger than the maximum \
hboisgon marked this conversation as resolved.
Show resolved Hide resolved
uparea in the grid {self.grid[self._MAPS['uparea']].max()}"
hboisgon marked this conversation as resolved.
Show resolved Hide resolved
)

rivdph_methods = ["gvf", "manning", "powlaw"]
if rivdph_method not in rivdph_methods:
raise ValueError(f'"{rivdph_method}" unknown. Select from {rivdph_methods}')
Expand Down
28 changes: 23 additions & 5 deletions hydromt_wflow/workflows/basemaps.py
Expand Up @@ -85,7 +85,7 @@ def hydrography(
outidx = None
if "mask" not in ds.coords and xy is None:
ds.coords["mask"] = xr.Variable(
dims=ds.raster.dims, data=np.ones(ds.raster.shape, dtype=np.bool)
dims=ds.raster.dims, data=np.ones(ds.raster.shape, dtype=bool)
)
elif "mask" not in ds.coords:
# NOTE if no subbasin mask is provided calculate it here
Expand All @@ -99,9 +99,27 @@ def hydrography(
# NOTE: this mask is passed on from get_basin_geometry method
logger.debug("Mask in dataset assumed to represent subbasins.")
ncells = np.sum(ds["mask"].values)
logger.debug(f"(Sub)basin at original resolution has {ncells} cells.")

scale_ratio = int(np.round(res / ds.raster.res[0]))

if ncells < 4:
raise ValueError(
"(Sub)basin at original resolution should at least consist of two cells on "
f"each axis and the total number of cells is {ncells}. "
"Consider using a larger domain or higher spatial resolution. "
"For subbasin models, consider a (higher) threshold on for example "
"upstream area or stream order to snap the outlet."
)
elif ncells < 100 and scale_ratio > 1:
logger.warning(
f"(Sub)basin at original resolution is small and has {ncells} cells. "
"This may results in errors later when upscaling flow directions. "
"If so, consider using a larger domain or higher spatial resolution. "
"For subbasin models, consider a (higher) threshold on for example "
"upstream area or stream order to snap the outlet."
)
else:
logger.debug(f"(Sub)basin at original resolution has {ncells} cells.")

if scale_ratio > 1: # upscale flwdir
if flwdir is None:
# NOTE initialize with mask is FALSE
Expand Down Expand Up @@ -154,7 +172,7 @@ def hydrography(
mask_int.raster.set_nodata(-1) # change nodata value
ds_out.coords["mask"] = mask_int.raster.reproject_like(
da_flw, method="nearest"
).astype(np.bool)
).astype(bool)
basins = ds_out["mask"].values.astype(np.int32)
logger.warning(
"The basin delineation might be wrong as no original resolution outlets"
Expand Down Expand Up @@ -251,7 +269,7 @@ def hydrography(
logger.debug(f"Outlet coordinates ({len(xy_pit[0])}/{npits}): {xy_pit_str}.")
if np.any(np.asarray(ds_out.raster.shape) == 1):
raise ValueError(
"The output extent should at consist of two cells on each axis. "
"The output extent should at least consist of two cells on each axis. "
hboisgon marked this conversation as resolved.
Show resolved Hide resolved
"Consider using a larger domain or higher spatial resolution. "
"For subbasin models, consider a (higher) threshold to snap the outlet."
)
Expand Down
2 changes: 1 addition & 1 deletion hydromt_wflow/workflows/river.py
Expand Up @@ -521,7 +521,7 @@ def _width_fit(
p0=[0.15, 0.65],
logger=logger, # rhine uparea based
):
outliers = np.full(np.sum(mask), False, dtype=np.bool)
outliers = np.full(np.sum(mask), False, dtype=bool)
a, b = None, None
# check if sufficient data
if np.sum(mask) > 10:
Expand Down
22 changes: 22 additions & 0 deletions tests/test_model_methods.py
Expand Up @@ -45,6 +45,28 @@ def test_setup_basemaps(tmpdir):

assert mod.grid["wflow_subcatch"].dtype == "int32"

# Test for too small basins
region = {"subbasin": [12.572061, 46.601984]}

with pytest.raises(ValueError) as error: # noqa PT011
mod.setup_basemaps(
region=region,
hydrography_fn=hydrography,
)
assert str(error.value).startswith(
"(Sub)basin at original resolution should at least consist of two cells"
)

region = {"subbasin": [12.572061, 46.600359]}
with pytest.raises(ValueError) as error: # noqa PT011
mod.setup_basemaps(
region=region,
hydrography_fn=hydrography,
)
assert str(error.value).startswith(
"The output extent should at least consist of two cells on each axis."
)


def test_setup_grid(example_wflow_model):
# Tests on setup_grid_from_raster
Expand Down