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] Add lake masking to stage- and flow-based CatFIM #1418

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

EmilyDeardorff
Copy link
Contributor

@EmilyDeardorff EmilyDeardorff commented Jan 31, 2025

Previously, CatFIM would inundate areas that we know to be lakes based on our FIM data. This update masks out lakes from stage- and flow-based CatFIM inundation.

Changes

  • inundation-mapping/tools/catfim/generate_categorical_fim_mapping.py: Added code to stage-based CatFIM to filter out HydroIDs that are associated with a non-null LakeID. Also uses new lake masking function to mask lakes from flow- and stage-based CatFIM right before the tifs are saved, at the end of produce_stage_based_lid_tifs(). Comments in this area were also cleaned up.
  • inundation-mapping/tools/tools_shared_functions.py: Added function that uses the water bodies geopackage to mask out lakes from a tiff.

Testing

  • Tested flow- and stage-based CatFIM at site auon6 (HUC 04140201)

  • Additional testing detailed in Issue 1358

  • Potential HUC/site combos where stage-based CatFIM intersects a lake (for additional testing):
    '07090002' 'okfi2'
    '07130008' 'kcai2'
    '07140202' 'ylow4'
    '10070001' 'brlk1'
    '11070204' 'tifm7'
    '11070208' 'olbo2'
    '11100301' 'wtnw2'
    '05020002' 'cpei3'
    '04300108' 'wlbn6'
    '04040002' 'racw3'

  • Potential HUC/site combos where flow-based CatFIM intersects a lake (for additional testing):

ahps_lid huc
hrho2 11100302
dibt2 12020002
lolt2 12030201

Site auon6 before update:
auon6_BEFORE_masking

Site auon6 AFTER update:
auon6_hydroids_filtered_lakes_masked

Deployment Plan (For developer use)

How does the changes affect the product?

  • Code only?
  • If applicable, has a deployment plan be created with the deployment person/team?
  • Require new or adjusted data inputs? Does it have start, end and duration code (in UTC)?
  • If new or updated data sets, has the FIM code been updated and tested with the new/adjusted data (subset is fine, but must be a subset of the new data)?
  • Require new pre-clip set?
  • Has new or updated python packages?

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
  • 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
  • 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

@EmilyDeardorff EmilyDeardorff added enhancement New feature or request CatFIM NWS Flood Categorical HAND FIM labels Jan 31, 2025
@EmilyDeardorff EmilyDeardorff self-assigned this Jan 31, 2025
@EmilyDeardorff EmilyDeardorff linked an issue Jan 31, 2025 that may be closed by this pull request
@EmilyDeardorff EmilyDeardorff changed the title WIP [1pt] Add lake masking to stage-based CatFIM [1pt] Add lake masking to stage-based CatFIM Feb 6, 2025
@EmilyDeardorff EmilyDeardorff marked this pull request as ready for review February 6, 2025 18:20
@RobHanna-NOAA
Copy link
Contributor

Testing for Stage Based looks great. Carson mentioned we should add this to Flow Based as well.

@EmilyDeardorff EmilyDeardorff changed the title [1pt] Add lake masking to stage-based CatFIM [1pt] Add lake masking to stage- and flow-based CatFIM Feb 10, 2025
@EmilyDeardorff
Copy link
Contributor Author

Implemented this change to flow-based and it looks like it's working as expected! I just need to do a bit more testing and then we should be good to go. The lake needing to be masked out from this example is in the bottom right corner. In the example with lake masking, we can see that inundation in the lake polygon has been removed.

auon6_04140201_without_lake_masking_flowbased

auon6_04140201_with_lake_masking_flowbased

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CatFIM NWS Flood Categorical HAND FIM enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[8pt] Add lake masking to stage-based CatFIM processing
3 participants