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

[1pt] PR: Clip ocean mask from DEM and extend outlet streams #1121

Merged
merged 12 commits into from May 6, 2024

Conversation

mluck
Copy link
Contributor

@mluck mluck commented Apr 15, 2024

Some NWM streams, particularly in coastal areas, fail to reach the edge of the DEM resulting in reverse flow (#916 (comment)). This issue was resolved by clipping the ocean mask from the buffered WBD and DEM, and any remaining streams that didn't have outlets reaching the edge of the buffered WBD boundary were extended by snapping the end to the nearest point on the buffered WBD.

Note: this does not solve the issue noted at NOAA-OWP/hydrofabric#38

Changes

  • data/wbd/clip_vectors_to_wbd.py: Clips landsea ocean mask from the buffered WBD and adds a function to extend outlet streams to the buffered WBD
  • data/wbd/clip_vectors_to_wbd.py: Updated multi-processing and added more logging.

Testing

DEM before (left) and after (right) clipping the ocean mask from the buffered WBD
image

Flow direction of DEM-derived reaches (red dashed lines) indicated by bold red arrows (before (left) and after (right))
image

Issuer Checklist (For developer use)

You may update this checklist before and/or after creating the PR. If you're unsure about any of them, please ask, we're here to help! These items are what we are going to look for before merging your code.

  • Informative and human-readable title, using the format: [_pt] PR: <description>
  • Links are provided if this PR resolves an issue, or depends on another other PR
  • If submitting a PR to the dev branch (the default branch), you have a descriptive Feature Branch name using the format: dev-<description-of-change> (e.g. dev-revise-levee-masking)
  • Changes are limited to a single goal (no scope creep)
  • The feature branch you're submitting as a PR is up to date (merged) with the latest dev branch
  • pre-commit hooks were run locally
  • Any change in functionality is tested
  • Passes all unit tests locally (inside interactive Docker container, at /foss_fim/, run: pytest unit_tests/)
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • CHANGELOG updated with template version number, e.g. 4.x.x.x
  • Reviewers requested
  • Add yourself as an assignee in the PR as well as the FIM Technical Lead

Merge Checklist (For Technical Lead use only)

  • Update CHANGELOG with latest version number and merge date
  • Update the Citation.cff file to reflect the latest version number in the CHANGELOG
  • If applicable, update README with major alterations

@mluck mluck added the bug Something isn't working label Apr 15, 2024
RobHanna-NOAA
RobHanna-NOAA previously approved these changes Apr 23, 2024
Copy link
Contributor

@RobHanna-NOAA RobHanna-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matt helped show me the before and after. Looks good.

Copy link
Contributor

@RobHanna-NOAA RobHanna-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During full scale testing, some errors occurred. It does appear that they only fail on HUC2 of 18 and higher but a full analysis was not done.

Here is an example of one fail (and each failing HUC has a different amount of the "Failed to auto identify EPSG: 7"

Full Error:
Get Vector Layers and Subset 19020103
Failed to auto identify EPSG: 7
Failed to auto identify EPSG: 7
Failed to auto identify EPSG: 7
Failed to auto identify EPSG: 7
Failed to auto identify EPSG: 7
Failed to auto identify EPSG: 7
Failed to auto identify EPSG: 7
Failed to auto identify EPSG: 7
Failed to auto identify EPSG: 7
Traceback (most recent call last):
File "//./foss_fim/data/wbd/generate_pre_clip_fim_huc8.py", line 329, in huc_level_clip_vectors_to_wbd
subset_vector_layers(
File "/foss_fim/data/wbd/clip_vectors_to_wbd.py", line 240, in subset_vector_layers
nwm_streams = extend_outlet_streams(nwm_streams, wbd_buffer, wbd)
File "/foss_fim/data/wbd/clip_vectors_to_wbd.py", line 64, in extend_outlet_streams
coords = [(coords) for coords in list(row['geometry'].coords)]
File "/usr/local/lib/python3.10/dist-packages/shapely/geometry/base.py", line 910, in coords
raise NotImplementedError(
NotImplementedError: Sub-geometries may have coordinate sequences, but multi-part geometries do not

Each HUC that had at least one EPSG: 7 failed with the sub-geometries error.

This is not a comprehensive list but some of the ones that failed are:18070305
18070201
19020401
18090101
18100203

and lots of others.
Doesn't appear there are any below 18's but not sure, might have been insufficent logging which was upgraded for the 18 and higher list.

@mluck
Copy link
Contributor Author

mluck commented Apr 30, 2024

Strange, I just ran a couple of the problem HUCs and didn't have any errors:

/foss_fim/data/wbd/generate_pre_clip_fim_huc8.py -n /data/inputs/pre_clip_huc8/24_03_29 -u /data/inputs/huc_lists/test_hucs.lst -j 6 -o 
Created directory: /data/inputs/pre_clip_huc8/24_03_29/18070305, huc level files will be written there.
Created directory: /data/inputs/pre_clip_huc8/24_03_29/18070201, huc level files will be written there.
Generating vectors for 18070305. 
Generating vectors for 18070201. 
Parallelizing HUC level wbd pre-clip vector creation. 

 Get WBD 18070305

 Get WBD 18070201

 - Creating -- /data/inputs/pre_clip_huc8/24_03_29/18070305/wbd.gpkg - Complete 

Get Vector Layers and Subset 18070305
Getting Cell Size for 18070305

 - Creating -- /data/inputs/pre_clip_huc8/24_03_29/18070201/wbd.gpkg - Complete 

Get Vector Layers and Subset 18070201
Getting Cell Size for 18070201
Create wbd buffer for 18070305
Create wbd buffer for 18070201
Create landsea gpkg for 18070305
Create landsea gpkg for 18070201
Subsetting Levee Protected Areas for 18070201
Subsetting Levee Protected Areas for 18070305
Subsetting NWM Lakes for 18070201
Subsetting NLD levee lines for 18070201
Subsetting NWM Lakes for 18070305
Subsetting NLD levee lines for 18070305
Subsetting NWM Headwater Points for 18070305
Subsetting NWM Headwater Points for 18070201
Subsetting NWM Catchments for 18070201
Subsetting NWM Streams for 18070201

         Completing Get Vector Layers and Subset: 18070201 

 Clip WBD 18070201
Subsetting NWM Catchments for 18070305

 - Creating -- /data/inputs/pre_clip_huc8/24_03_29/18070201/wbd.gpkg - Complete
                 run time for huc 18070201: is 0:02:00
Subsetting NWM Streams for 18070305

         Completing Get Vector Layers and Subset: 18070305 

 Clip WBD 18070305

 - Creating -- /data/inputs/pre_clip_huc8/24_03_29/18070305/wbd.gpkg - Complete
                 run time for huc 18070305: is 0:03:12

         Completed writing all huc level files 

                 TOTAL RUN TIME: 0:03:13

RobHanna-NOAA
RobHanna-NOAA previously approved these changes May 2, 2024
@CarsonPruitt-NOAA CarsonPruitt-NOAA merged commit ad51dc2 into dev May 6, 2024
1 check passed
@CarsonPruitt-NOAA CarsonPruitt-NOAA deleted the dev-extend-outlets branch May 6, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants