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: Catchment Boundary Tool Addition #1149

Merged
merged 26 commits into from
Sep 13, 2024

Conversation

RileyMcDermott-NOAA
Copy link
Contributor

@RileyMcDermott-NOAA RileyMcDermott-NOAA commented May 7, 2024

This PR adds scripts that can identify areas within produced inundation rasters where glasswalling of inundation occurs due to catchment boundaries, know as catchment boundary issues.

Addresses #1063.

Additions

  • tools/identify_catchment_boundary.py: Identifies where catchment boundaries are glasswalling inundation extent.

  • tools/inundate_catchment_boundary.py: Produces inundation for given HUC and identifies catchment boundary issues in produced FIM.

Testing

  • Tested successfully on HUC 17110009 and 17110010.

Screenshots

Examples of HUC 17110010 from #1063:

image

image

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

@RileyMcDermott-NOAA RileyMcDermott-NOAA added the enhancement New feature or request label May 7, 2024
@RileyMcDermott-NOAA RileyMcDermott-NOAA changed the title Dev catchment boundary tool [1pt] PR: Catchment Boundary Tool Addition May 7, 2024
@RileyMcDermott-NOAA RileyMcDermott-NOAA marked this pull request as ready for review May 21, 2024 17:37
@RyanSpies-NOAA
Copy link
Collaborator

I briefly tested the code tools/inundate_catchment_boundary.py for huc 12090301 using the 50yr AEP flow file. The process took 6.69 minutes using 7 cores on my ec2 machine. The boundary_output.gpkg is correctly identifying all instances with coincident boundaries between the inundation extent and any of the catchment boundaries.

image

Some future consideration notes:
It may make sense to set a length threshold on the boundary output segments to help weed out "benign" boundaries. I've noted numerous instances with very small (<100m) boundary segments. Many of these small boundary segments appear to be an "artifact" of having slightly different inundation extents along the branch 0 catchment boundaries (this is more of a WSE smoothing issue rather than catchment boundary).
image

image

It would also be nice to have an attribute stored with the boundary lines that identified whether a segment applies to branch 0, the GMS branch, or both. In my opinion, the most egregious and impactful glass walling issues often coincide with locations where both branch 0 and the GMS branch share a catchment boundary so it would be awesome if we could filter to just view those cases.

Looks like the boundary line segments are spliced according to the 10m pixel boundaries - 12090301 currently shows 36k+ line segments in the boundary gpkg. I suggest joining the connected lines together in grouped segments to make the lines more manageable if we anticipate exposing any metadata with the lines.
image

Copy link
Contributor

@ZahraGhahremani ZahraGhahremani left a comment

Choose a reason for hiding this comment

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

I tested the tools/inundate_catchment_boundary.py script for several HUCs (02030105, 21010005, 15020001, and 01060001). Each run took approximately 2 to 3 minutes, and the results successfully identified the intersection between the raster extent and the catchment boundary.
However, I suggest adding an attribute to the boundary lines to indicate whether a segment applies to branch 0, the GMS branch, or both (As Ryan mentioned too). This would help prevent confusion at first glance. For example, in HUC 02030105, there is a boundary line in branch 0 even though there is no intersection between the raster and the catchment. This initially seems confusing but makes sense when examining other branch boundaries.

Example of HUC 02030105, branch 0:
0

Example of HUC 02030105, branch 6264000012:
GMS

RobHanna-NOAA
RobHanna-NOAA previously approved these changes Jul 19, 2024
@RobHanna-NOAA
Copy link
Contributor

still being worked on

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.

Still wip

@RileyMcDermott-NOAA
Copy link
Contributor Author

Updated code to link catchment boundary lines correct HydroID:

  • Error lines labeled with HydroID
  • Catchment polygons labeled with HydroID

image

Updated code to identify which branch is causing the error:

  • Error lines labeled with branch number
  • Catchment polygons from branch 0 labeled with HydroID
  • Overlapping error (same error area from multiple branches) creates one error line for each branch, shown in the overlapping yellow and purple lines.

image

Additionally filtered out all error lines less than 100m.

RobHanna-NOAA
RobHanna-NOAA previously approved these changes Sep 13, 2024
@CarsonPruitt-NOAA CarsonPruitt-NOAA merged commit 4e4ba68 into dev Sep 13, 2024
1 check passed
@CarsonPruitt-NOAA CarsonPruitt-NOAA deleted the dev-catchment-boundary-tool branch September 13, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Hold
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[21pt] Create a tool for identifying catchment boundary issues
5 participants