From 80b9d95cbb6b22c92b8383b5e0635a5dd69de4c1 Mon Sep 17 00:00:00 2001 From: lou-a Date: Mon, 30 Sep 2024 15:27:25 -0400 Subject: [PATCH 1/7] Update routing_product.py Modified routing_product.py to correctly set the river length to zero and set default values for reach attributes in sub-basins with no channel routing (i.e., sub-basins with lakes or headwater basins). --- src/ravenpy/extractors/routing_product.py | 35 ++++++++++++++++------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/ravenpy/extractors/routing_product.py b/src/ravenpy/extractors/routing_product.py index 1505f570..6837e6eb 100644 --- a/src/ravenpy/extractors/routing_product.py +++ b/src/ravenpy/extractors/routing_product.py @@ -148,7 +148,13 @@ def _extract_subbasin(self, row, is_lake, subbasin_ids) -> dict: riv_length_field = ( "Rivlen" if self.routing_product_version == "1.0" else "RivLength" ) - river_length_in_kms = 0 if is_lake else round(row[riv_length_field] / 1000, 5) + # Correctly setting the river length to zero for sub-basins with no channel routing, such as sub-basins with lakes or headwater basins + if row[riv_length_field] > 1.0: + river_length_in_kms = round(row[riv_length_field] / 1000, 5) + else: + river_length_in_kms = 0 + if is_lake: + river_length_in_kms = 0 # river_slope = max( # row["RivSlope"], RoutingProductShapefileExtractor.MAX_RIVER_SLOPE # ) @@ -192,15 +198,24 @@ def _extract_reservoir(row) -> dict: @staticmethod def _extract_channel_profile(row) -> dict: subbasin_id = int(row["SubId"]) - slope = max(row["RivSlope"], BasinMakerExtractor.MAX_RIVER_SLOPE) - - # SWAT: top width of channel when filled with water; bankfull width W_bnkfull - channel_width = max(row["BkfWidth"], 1) - # SWAT: depth of water in channel when filled to top of bank - channel_depth = max(row["BkfDepth"], 1) - channel_elev = row["MeanElev"] - floodn = row["FloodP_n"] - channeln = row["Ch_n"] + + # Correctly defining the reach attributes for sub-basins with no channel routing (i.e., lakes or headwater basins) + if row["RivLength"] > 1.0: + slope = max(row["RivSlope"], BasinMakerExtractor.MAX_RIVER_SLOPE) + # SWAT: top width of channel when filled with water; bankfull width W_bnkfull + channel_width = max(row["BkfWidth"], 1) + # SWAT: depth of water in channel when filled to top of bank + channel_depth = max(row["BkfDepth"], 1) + channel_elev = row["MeanElev"] + floodn = row["FloodP_n"] + channeln = row["Ch_n"] + else: + slope = 0.12345 + channeln = 0.12345 + floodn = 0.12345 + channel_width = 0.12345 + channel_depth = 0.12345 + channel_elev = row["MeanElev"] # channel profile calculations are based on theory SWAT model is based on # see: https://swat.tamu.edu/media/99192/swat2009-theory.pdf From 31bd68d227e027c78aa7d102aa5dc0db5baebb2f Mon Sep 17 00:00:00 2001 From: lou-a Date: Mon, 30 Sep 2024 15:27:25 -0400 Subject: [PATCH 2/7] Update routing_product.py Modified routing_product.py to correctly set the river length to zero and set default values for reach attributes in sub-basins with no channel routing (i.e., sub-basins with lakes or headwater basins). --- src/ravenpy/extractors/routing_product.py | 35 ++++++++++++++++------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/ravenpy/extractors/routing_product.py b/src/ravenpy/extractors/routing_product.py index 1505f570..6837e6eb 100644 --- a/src/ravenpy/extractors/routing_product.py +++ b/src/ravenpy/extractors/routing_product.py @@ -148,7 +148,13 @@ def _extract_subbasin(self, row, is_lake, subbasin_ids) -> dict: riv_length_field = ( "Rivlen" if self.routing_product_version == "1.0" else "RivLength" ) - river_length_in_kms = 0 if is_lake else round(row[riv_length_field] / 1000, 5) + # Correctly setting the river length to zero for sub-basins with no channel routing, such as sub-basins with lakes or headwater basins + if row[riv_length_field] > 1.0: + river_length_in_kms = round(row[riv_length_field] / 1000, 5) + else: + river_length_in_kms = 0 + if is_lake: + river_length_in_kms = 0 # river_slope = max( # row["RivSlope"], RoutingProductShapefileExtractor.MAX_RIVER_SLOPE # ) @@ -192,15 +198,24 @@ def _extract_reservoir(row) -> dict: @staticmethod def _extract_channel_profile(row) -> dict: subbasin_id = int(row["SubId"]) - slope = max(row["RivSlope"], BasinMakerExtractor.MAX_RIVER_SLOPE) - - # SWAT: top width of channel when filled with water; bankfull width W_bnkfull - channel_width = max(row["BkfWidth"], 1) - # SWAT: depth of water in channel when filled to top of bank - channel_depth = max(row["BkfDepth"], 1) - channel_elev = row["MeanElev"] - floodn = row["FloodP_n"] - channeln = row["Ch_n"] + + # Correctly defining the reach attributes for sub-basins with no channel routing (i.e., lakes or headwater basins) + if row["RivLength"] > 1.0: + slope = max(row["RivSlope"], BasinMakerExtractor.MAX_RIVER_SLOPE) + # SWAT: top width of channel when filled with water; bankfull width W_bnkfull + channel_width = max(row["BkfWidth"], 1) + # SWAT: depth of water in channel when filled to top of bank + channel_depth = max(row["BkfDepth"], 1) + channel_elev = row["MeanElev"] + floodn = row["FloodP_n"] + channeln = row["Ch_n"] + else: + slope = 0.12345 + channeln = 0.12345 + floodn = 0.12345 + channel_width = 0.12345 + channel_depth = 0.12345 + channel_elev = row["MeanElev"] # channel profile calculations are based on theory SWAT model is based on # see: https://swat.tamu.edu/media/99192/swat2009-theory.pdf From 0a06ed3b24747b906ecfc0af6892ef81405dfae8 Mon Sep 17 00:00:00 2001 From: Louise Arnal Date: Wed, 2 Oct 2024 10:11:19 -0400 Subject: [PATCH 3/7] Apply suggestions from code review Co-authored-by: David Huard --- src/ravenpy/extractors/routing_product.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/ravenpy/extractors/routing_product.py b/src/ravenpy/extractors/routing_product.py index 6837e6eb..35da7a80 100644 --- a/src/ravenpy/extractors/routing_product.py +++ b/src/ravenpy/extractors/routing_product.py @@ -149,12 +149,10 @@ def _extract_subbasin(self, row, is_lake, subbasin_ids) -> dict: "Rivlen" if self.routing_product_version == "1.0" else "RivLength" ) # Correctly setting the river length to zero for sub-basins with no channel routing, such as sub-basins with lakes or headwater basins - if row[riv_length_field] > 1.0: - river_length_in_kms = round(row[riv_length_field] / 1000, 5) - else: - river_length_in_kms = 0 - if is_lake: + if is_lake or row[riv_length_field] <= 1.0: river_length_in_kms = 0 + else: + river_length_in_kms = round(row[riv_length_field] / 1000, 5) # river_slope = max( # row["RivSlope"], RoutingProductShapefileExtractor.MAX_RIVER_SLOPE # ) @@ -200,13 +198,13 @@ def _extract_channel_profile(row) -> dict: subbasin_id = int(row["SubId"]) # Correctly defining the reach attributes for sub-basins with no channel routing (i.e., lakes or headwater basins) + channel_elev = row["MeanElev"] if row["RivLength"] > 1.0: slope = max(row["RivSlope"], BasinMakerExtractor.MAX_RIVER_SLOPE) # SWAT: top width of channel when filled with water; bankfull width W_bnkfull channel_width = max(row["BkfWidth"], 1) # SWAT: depth of water in channel when filled to top of bank channel_depth = max(row["BkfDepth"], 1) - channel_elev = row["MeanElev"] floodn = row["FloodP_n"] channeln = row["Ch_n"] else: @@ -215,7 +213,6 @@ def _extract_channel_profile(row) -> dict: floodn = 0.12345 channel_width = 0.12345 channel_depth = 0.12345 - channel_elev = row["MeanElev"] # channel profile calculations are based on theory SWAT model is based on # see: https://swat.tamu.edu/media/99192/swat2009-theory.pdf From 0b112677da07d64756d0ed0a154ca6276f063832 Mon Sep 17 00:00:00 2001 From: lou-a Date: Wed, 2 Oct 2024 13:04:42 -0400 Subject: [PATCH 4/7] Updated the change log, added contributor information and included tests Added a note in the CHANGELOG.rst, added contributor information in the .zenodo.json and in the AUTHORS.rst files, and added tests in the test_extractor.py script to confirm that everything is working as I expect. --- .zenodo.json | 7 ++++++- AUTHORS.rst | 2 +- CHANGELOG.rst | 1 + tests/test_extractor.py | 14 ++++++++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/.zenodo.json b/.zenodo.json index 326f0641..29b8093c 100644 --- a/.zenodo.json +++ b/.zenodo.json @@ -24,7 +24,12 @@ "name": "Arsenault, Richard", "affiliation": "École de technologie supérieure", "orcid": "0000-0003-2834-2750" - } + }, + { + "name": "Arnal, Louise", + "affiliation": "Ouranos", + "orcid": "0000-0002-0208-2324" + }, ], "keywords": [ "climate change", diff --git a/AUTHORS.rst b/AUTHORS.rst index c46ee369..e996529c 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -18,4 +18,4 @@ Co-Developers Contributors ------------ -None yet. Why not be the first? +* Louise Arnal `@lou-a `_ diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 94a0a02d..35a0cf83 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -18,6 +18,7 @@ Internal changes * `ravenpy` now has a `CODE_OF_CONDUCT.md` file. * Many `numpydoc`-style docstrings have been adjusted for consistency. * Added `setuptools` to the `gis` build recipe to ensure that the `gdal` bindings are built successfully. (PR #400) +* Modified the sub-basin and channel profile extraction functions to correctly set the river length to zero and set default values for reach attributes in sub-basins with no channel routing (i.e., sub-basins with lakes or headwater basins). (PR #401) v0.15.0 (2024-06-20) -------------------- diff --git a/tests/test_extractor.py b/tests/test_extractor.py index f1411d0f..db66da1f 100644 --- a/tests/test_extractor.py +++ b/tests/test_extractor.py @@ -15,7 +15,21 @@ def test_basinmaker_extractor(get_local_testdata, tmp_path): routing_product_version="2.1", ) rvh_config = rvh_extractor.extract(hru_from_sb=True) + + # Create lists of values to check + bedslope_list = [item['bed_slope'] for item in rvh_config["channel_profile"]] + mannings_list = [value for d in rvh_config["channel_profile"] for value in [t[1] for t in d['roughness_zones']]] + reach_length_list = [item['reach_length'] for item in rvh_config["sub_basins"]] + rvh_config.pop("channel_profile") config = BasicRoute(**rvh_config) config.write_rv(tmp_path, modelname="routing") + + # Checks that the bedslope, Manning and reach length values are non negative numbers + assert all(isinstance(x, (int, float)) for x in bedslope_list) is True + assert any(x < 0 for x in bedslope_list) is False + assert all(isinstance(y, (int, float)) for y in mannings_list) is True + assert any(y < 0 for y in mannings_list) is False + assert all(isinstance(z, (int, float)) for z in bedslope_list) is True + assert any(z < 0 for z in reach_length_list) is False From f6575d967053a3bbc90c42a9313ee74b5ccf5903 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 2 Oct 2024 17:05:25 +0000 Subject: [PATCH 5/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_extractor.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_extractor.py b/tests/test_extractor.py index db66da1f..b822e8bc 100644 --- a/tests/test_extractor.py +++ b/tests/test_extractor.py @@ -17,9 +17,13 @@ def test_basinmaker_extractor(get_local_testdata, tmp_path): rvh_config = rvh_extractor.extract(hru_from_sb=True) # Create lists of values to check - bedslope_list = [item['bed_slope'] for item in rvh_config["channel_profile"]] - mannings_list = [value for d in rvh_config["channel_profile"] for value in [t[1] for t in d['roughness_zones']]] - reach_length_list = [item['reach_length'] for item in rvh_config["sub_basins"]] + bedslope_list = [item["bed_slope"] for item in rvh_config["channel_profile"]] + mannings_list = [ + value + for d in rvh_config["channel_profile"] + for value in [t[1] for t in d["roughness_zones"]] + ] + reach_length_list = [item["reach_length"] for item in rvh_config["sub_basins"]] rvh_config.pop("channel_profile") From a07dbaaf1e21d1c298b9d8cf76e55369bf0d0201 Mon Sep 17 00:00:00 2001 From: Louise Arnal Date: Wed, 2 Oct 2024 13:37:06 -0400 Subject: [PATCH 6/7] Update .zenodo.json --- .zenodo.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.zenodo.json b/.zenodo.json index 29b8093c..2cea0625 100644 --- a/.zenodo.json +++ b/.zenodo.json @@ -29,7 +29,7 @@ "name": "Arnal, Louise", "affiliation": "Ouranos", "orcid": "0000-0002-0208-2324" - }, + } ], "keywords": [ "climate change", From 1d767db63e44da918ceea5ae922c47581f71dc6e Mon Sep 17 00:00:00 2001 From: Louise Arnal Date: Wed, 2 Oct 2024 13:38:15 -0400 Subject: [PATCH 7/7] Update CHANGELOG.rst --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 35a0cf83..ab763786 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -18,7 +18,7 @@ Internal changes * `ravenpy` now has a `CODE_OF_CONDUCT.md` file. * Many `numpydoc`-style docstrings have been adjusted for consistency. * Added `setuptools` to the `gis` build recipe to ensure that the `gdal` bindings are built successfully. (PR #400) -* Modified the sub-basin and channel profile extraction functions to correctly set the river length to zero and set default values for reach attributes in sub-basins with no channel routing (i.e., sub-basins with lakes or headwater basins). (PR #401) +* Modified the sub-basin and channel profile extraction functions to correctly set the river length to zero and set default values for reach attributes in sub-basins with no channel routing (i.e., sub-basins with lakes or headwater basins). (issue #354, PR #401) v0.15.0 (2024-06-20) --------------------