Skip to content

Conversation

@robgpita
Copy link
Contributor

@robgpita robgpita commented Sep 30, 2025

PR - NGWPC PI7 Delivery

Modifications and additions to bring in NGWPC work in PI3 - PI7, including: Pulling 3dep DEM Tiles, Variable HUC Level Parameterization (HLP), HPC Capabilities (Slurm), USGS High Water Mark & Ripple1d SRC incorporation.

Description

OWP’s latest main branch (v4.8.7.3 3c237df) has been merged, extensive comments have been inserted where breaking changes to exisitng NWCPC functionality occur, and where future resolutions are likely. The branch submitted as this Pull Request has been modified to prioritize HLP, and the existing pre clip directories (s3://fimc-data/hand_fim/inputs/pre_clip/25_05_10_huc${huc_level}). This required reverting certain sections of code to the 5/22 Release (v4.6.1.4, 84b5ae3). Searching for the string v4.8.7.3 should reveal the majority of the breaking changes, with their resolutions. There is a possibility that some resolutions were made without including the v4.8.7.3 string in a comment, so these resolutions may not be fully inclusive. See Outstanding Items.

PR #1206 introduced modifications that are incompatible with HLP and USGS HWM functionality. All SRC and post processing steps require further investigation with regards to full compatibility and accuracy, given the divergence with new modules introduced in both NGWPC and OWP codebases.

The current pre clip directories in use (pre_clip_huc_dir in src/bash_variables.env) do not have newest bridge data. To enable the use of this codebase using existing pre clip directories, routines to incorporate updated osm bridge data have been reverted to 5/22 version of codebase. All osm bridge functionality should be reviewed and tested, as there was significant divergence. A good faith attempt was make to keep this functionality intact, but due to the discrepancy in input data, this component will likely require further resolutions.

Files and directories which exist in the dev-NGWPC branch, but were removed in main (v4.8.7.3), persisted beyond the merge. The unit_tests/ folder, and config/aws_s3_put_fim3_hydrovis_whitelist.lst file were deleted to match what is in the main branch. There may be additional files and folder deletions stemming from the divergence which were not identified and removed.

Additions

  • data/
    • nws/
      • .gitignore
    • usgs/
      • clip_tile_indices.py
      • create_3dep_tile_index.py
      • get_3dep_static_tiles.py
    • wbd/
      • generate_pre_clip_fim_huc.py
      • get_child_or_parent_hucs_for_huc_list.py
      • hucs.py
  • slurm/
    • check_arguments.sh
    • README.md
    • slurm_partition_process_unit.sh
    • slurm_pipeline.sh
    • slurm_post_processing.sh
    • slurm_pre_processing.sh
    • slurm_process_unit_wb_partitions.sh
    • slurm_process_unit_wb_restart.sh
    • slurm_process_unit_wb.sh
    • slurm_sierra_test.sh
    • slurm_single_fim_pipeline.sh
  • src/
    • fill_depressions.py
    • src_adjust_ripple1d_rating.py
  • tools/
    • acquire_tigerweb_data.py
    • convert_city_names_to_huc8s.py
    • create_completion_maps.py
    • partition_and_write_parquet.py

Changes

  • .gitignore
  • Dockerfile.dev
  • Pipfile
  • Pipfile.lock
  • README.md
  • config/
    • deny_branches.lst
    • deny_unit.lst
    • params_template.env
  • data/
    • wbd/
      • generate_pre_clip_fim_huc8.py
      • preprocess_wbd.py
  • fim_pipeline.sh
  • fim_post_processing.sh
  • fim_pre_processing.sh
  • fim_process_unit_wb.sh
  • pyproject.toml
  • src/
    • accumulate_headwaters.py
    • add_crosswalk.py
    • adjust_thalweg_lateral.py
    • aggregate_by_huc.py
    • agreedem.py
    • bash_functions.env
    • bash_variables.env
    • bathymetric_adjustment.py
    • build_stream_traversal.py
    • check_huc_inputs.py
    • delineate_hydros_and_produce_HAND.sh
    • filter_catchments_and_add_attributes.py
    • generate_branch_list_csv.py
    • heal_bridges_osm.py
    • identify_src_bankfull.py
    • make_rem.py
    • mask_dem.py
    • process_branch.sh
    • run_unit_wb.sh
    • split_flows.py
    • src_adjust_ras2fim_rating.py
    • src_adjust_spatial_obs.py
    • src_adjust_usgs_rating_trace.py
    • src_roughness_optimization.py
    • stream_branches.py
    • subdiv_chan_obank_src.py
    • unique_pixel_and_allocation.py
    • update_htable_src.py
    • usgs_gage_crosswalk.py
    • usgs_gage_unit_setup.py
    • utils/
      • shared_functions.py
  • tools/
    • combine_crosswalk_tables.py
    • hash_compare.py
    • run_test_case.py
    • tools_shared_functions.py

Removals

Testing

The code proposed as part of this branch has been tested, and outputs are on S3: s3://fimc-data/hand_fim/outputs/merge_NGWPC_and_v4_8_7_3/HUC<>s_MERGED.
There are three HUC levels and three versions tested. These outputs were generated to test and compare the original NGWPC branch (dev-NGWPC), identified with the naming pattern HUC<>s_NGWPC. The main branch, identified with the naming pattern HUC8s_v4_8_7_3. And the merged branch (dev-NGWPC-merge-main-temp), identified with the naming pattern HUC<>s_MERGED.

Methods

The process taken for this pull request is as follows:

1.) Checkout dev-NGWPC branch

2.) Create a new branch named dev-NGWPC-merge-main

3.) Merge main (v4.8.7.3 commit: 3c237df758a8055c14bfe9af36d425fc336c474c)

4.) Resolve all merge conflicts, with a focus on keeping NGWPC functionality intact
Commit (055ae80805549620e263fdd4355f2045c21bd608) and push this branch to NGWPC fork

5.) From dev-NGWPC-merge-main, checkout a new branch named dev-NGWPC-merge-main-temp

6.) Continued merge conflicts and resolutions in this branch, focusing on the enablement of NGWPC code to successfully complete processing without any errors.

7.) Made commits directly to this branch

  • Build new docker image and create new Pipfile.lock based on updated Pipfile
  • Modify variables (src/bash_variables.env) to reference HLP data on PW EFS
  • Revert bridges and bathymetric routines to 5/22 release (v4.6.1.4)
  • Remove the call to convert_to_int16.py
  • Adhere code to style guidelines (black, isort, flake8)
  • Remove unit_tests/ directory (directory not deleted as part of merge)
  • Add a patch for HUC10s failing aggregrate_by_huc.py
  • Update slurm/README.md

8.) Create a patch with all changes, and condense into a single commit:

  • git diff main..dev-NGWPC-merge-main-temp > main-merge-NGWPC.patch
  • git checkout main
  • git checkout -b main-v_4_8_7_3-NGWPC-merge
  • git apply main-merge-NGWPC.patch
  • rm main-merge-NGWPC.patch
  • git add .
  • git commit --no-verify
        Incorporate all NGWPC work (HLP, Slurm, HWM, Ripple1d) 
        Resolve merge conflicts.
    
        Co-authored-by: fernando-aristizabal <[email protected]>
        Co-authored-by: BradfordBates-NOAA <[email protected]>
        Co-authored-by: cwlyden <[email protected]>
  • git push -u origin main-v_4_8_7_3-NGWPC-merge

9.) Issue Pull Request

Notes on Commit History

Due to the significant divergence in commit histories, the preferred operation of git rebase main was not practical. Attempts were made to cherry-pick commits from NGWPC work and apply them to new branch checked out from main, but this does not remove the necesity to resolve all merge conflicts. Therefore, a patch was created, and all the NGWPC commits were consolidated in one commit, using the process outlined above.

Outstanding Items

Searching for the string v4.8.7.3 should reveal locations where adjustments were needed to enable HLP with existing input data. Below should not be considered a comprehensive list, as there may be other locations where adjustments are necessary, namely in post-processing steps.

The following files contain conflicts, where a resolution was made to keep HLP functionality intact and/or suggest HLP incorporation:

  • data/wbd/generate_pre_clip_fim_huc8.py
  • src/add_crosswalk.py
  • src/aggregate_by_huc.py
  • src/bash_variables.env
  • src/bathymetric_adjustment.py
  • src/delineate_hydros_and_produce_HAND.sh
  • src/heal_bridges_osm.py
  • src/adjust_spatial_obs.py

The file that generates variable HUC level pre clip directories (genererate_pre_clip_fim_huc.py) will require modification to match the new subset_vector_layers function in clip_vectors_to_wbd.py.

To use existing HUC8 level pre clip directories with this version, the only change necessary is to rename the wbd8_clp.gpkg file in the HUC8 level pre clip to wbd_clp.gpkg.

A reconciliation of the HydroID to enable HLP should be investigated in src/add_crosswalk.py. For now, the call to src/convert_to_int16.py in src/delineate_hydros_and_produce_HAND.sh has been commented to enable HLP HydroIDs.

New methods related to updated osm bridge data within the pre clip directories is divergent, most all bridge related functionality was reverted to v4.6.1.4 or earlier.

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

Resolve merge conflicts.

Co-authored-by: fernando-aristizabal <[email protected]>
Co-authored-by: BradfordBates-NOAA <[email protected]>
Co-authored-by: cwlyden <[email protected]>
@RobHanna-NOAA RobHanna-NOAA added the enhancement New feature or request label Sep 30, 2025
@robgpita robgpita marked this pull request as ready for review September 30, 2025 19:11
@DJackson2313
Copy link

SWCM witness approval; release concurrence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants