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: update SRC optimization w/ ras2fim v2 inputs #1247

Merged
merged 11 commits into from
Aug 16, 2024

Conversation

RyanSpies-NOAA
Copy link
Collaborator

@RyanSpies-NOAA RyanSpies-NOAA commented Aug 8, 2024

Updated the gauge crosswalk and SRC adjustment routine to use the ras2fim v2 files. The v2 ras2fim file structure was changed to organize the data by huc8 - one gpkg and csv per huc8. Addresses #1091

Changes

  • fim_post_processing.sh: added new input variables for running the src_adjust_ras2fim_rating.py
  • src/bash_variables.env: renamed and reassigned the ras2fim input variables: ras2fim_input_dir, ras_rating_curve_csv_filename, ras_rating_curve_gpkg_filename
  • src/run_unit_wb.sh: Added logic to check if huc in process has ras2fim input data to process. If yes - copy the ras2fim cross section point gpkg to the huc run directory.
  • src/src_adjust_ras2fim_rating.py: Updated code logic to use the huc-specific input files containing the ras2fim rating curve data (previous ras2fim input file contained all hucs in one csv)
  • src/utils/shared_functions.py: Added function to find huc subdirectories with the same name btw two parent folders

Testing

I ran fim_pipeline.sh for the 12 huc8s that have v2 ras2fim data.
image

Alpha Test results show a modest improvement for 4 BLE sites:
image

Notes

The new ras2fim v2 input files are already available in all the necessary systems (/inputs/rating_curve/ras2fim_exports/v2_0/)

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

@RyanSpies-NOAA RyanSpies-NOAA self-assigned this Aug 8, 2024
- ras2fim rating curves have error in the cms variable
@RyanSpies-NOAA RyanSpies-NOAA linked an issue Aug 9, 2024 that may be closed by this pull request
@RyanSpies-NOAA RyanSpies-NOAA changed the title WIP - PR update SRC optimization w/ ras2fim v2 inputs [1pt] PR: update SRC optimization w/ ras2fim v2 inputs Aug 12, 2024
@RyanSpies-NOAA RyanSpies-NOAA marked this pull request as ready for review August 12, 2024 17:05
hhs732
hhs732 previously approved these changes Aug 13, 2024
Copy link
Contributor

@hhs732 hhs732 left a comment

Choose a reason for hiding this comment

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

This PR was tested on 3 HUC8 of 11010004, 12030109, and 12040101. The fim_pipline was successfully ran and synthesize_testcases.py also was ran successfully. The results of this test is availble on /efs-drives/fim-dev-efs/fim-home/heidi.safa/projects/dev-ras2fim-calb-update/outputs/pr_ras2fim/

hhs732
hhs732 previously approved these changes Aug 16, 2024
Copy link
Contributor

@hhs732 hhs732 left a comment

Choose a reason for hiding this comment

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

Changes in the catalog approved.

@CarsonPruitt-NOAA CarsonPruitt-NOAA merged commit c20e897 into dev Aug 16, 2024
1 check passed
@CarsonPruitt-NOAA CarsonPruitt-NOAA deleted the dev-ras2fim-calb-update branch August 16, 2024 19:06
@RyanSpies-NOAA RyanSpies-NOAA linked an issue Sep 11, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants