[1pt] PR: Update benchmark flows in AHPS domain

[mluck]

Extends flows (i.e., discharge) to stream segments missing from NWS or USGS validation flow files. The levelpath associated with existing flows in the AHPS domain is identified, and any stream segments of the levelpath in the domain missing from the flow file are added to the flow file by assigning the existing flow (this is a constant value regardless of other tributaries including other levelpaths in the domain).

Closes #1089 and closes #1092.

Additions

• data/extend_benchmark_flows.py: Adds missing flows to NWS or USGS benchmark flow files. The original flow file is saved with an appended .bak.

Changes

• tools/tools_shared_variables.py: Removed corrected flow files from BAD_SITES list.

Testing

Verified that the new flow files contain the added feature_id/discharge values from missing stream segments. Also ran synthesize_test_cases.py to generate inundation agreement rasters.

Example results from sweg1 (HUC8 03130001, major flow category)

Flow file (added segments in red)

Inundation agreement raster (before/after)

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]

Since the AHPS benchmark flows seem to be (always?) constant within a flow file and there are other sites that seem to be missing flows in the domain (e.g., chin7), what about applying this to all of the AHPS benchmark flows rather than just the ones in the BAD_SITES list?

[CarsonPruitt-NOAA]

That makes sense to me.

[CarsonPruitt-NOAA]

@mluck Can you add cart2 to the BAD_SITES list? @derekgiardino has identified this a site on which we shouldn't be doing evaluations.

[mluck]

Updated so that all NWS and USGS benchmark flows are updated (not just those in BAD_SITES).

An input to this is a FIM run (e.g., BED) since it uses the derived levelpaths.

[RyanSpies-NOAA]

I have a new error -->
Traceback (most recent call last):
File "//foss_fim/data/update_benchmark_flows.py", line 134, in
update_benchmark_flows(**args)
File "//foss_fim/data/update_benchmark_flows.py", line 81, in update_benchmark_flows
assert os.path.exists(levelpath_file), f"Levelpath file {levelpath_file} does not exist"
AssertionError: Levelpath file data/previous_fim/fim_4_4_5_0/04300109/nwm_subset_streams_levelPaths.gpkg does not exist

[mluck]

Previously, it was checking to make sure the input file existed, and if it didn't then it threw the AssertionError. Now, if an input file doesn't exist for a location, updating the flows are skipped for that location.

I was able to run this PR on the full set of AHPS benchmarks using fim_4_4_0_0.

[RyanSpies-NOAA]

Successfully ran the script for FIM 4.4.3.0

[RyanSpies-NOAA]

Confirmed that CBST2 flow file is unchanged with this feature branch. The flow file currently includes erroneous main stem flow values for the tributary river reaches (order=4). Will likely need a new ticket to address this.

[RyanSpies-NOAA]

RGDT2 (13090001) shows a slight regression with the new flow file. It appears that the tributary was erroneously added to the flow file -->

[mluck]

It looks like that's happening because the tributary is where the levelpath is designated. The levelpaths appear to be incorrect as the green levelpath is order 8 as well as the connected mainstem segment of the blue levelpath, but the tributary portion of the blue levelpath is order 4.

[mluck]

The stream segments are disconnected which is why the flow is being continued up the tributary instead of the mainstem

[mluck]

Would it make sense to add a feature to remove flows from feature_ids not on the mainstem levelpath (i.e., tributaries) as part of this PR?

[CarsonPruitt-NOAA]

Yeah, I think we could include removing the tributaries as part of this PR. It'll be sort of a benchmark flows overhaul.

[RyanSpies-NOAA]

Couple more plots to add here for future reference. The aggregate alpha test scores (NWS & USGS sites) show a nice MCC and CSI improvement as expected:

[mluck]

Flows in tributaries (not on the main levelpath) are now removed from the flow file. Example: flows from cbst2 NWS moderate magnitude (left: original, right: corrected)

[CarsonPruitt-NOAA]

Wow, that's a pretty major reduction in feature_ids. Did you confirm that these are accurate?

[mluck]

Yeah I checked that it's accurate. In addition to the tributary segments, a lot of the segments that are removed are up- and downstream of the AHPS domain (this seems to be the case for most of the flow files).

[RyanSpies-NOAA]

With Matt's latest change nearly all of the flow files have been modified (as expected):

[mluck]

kcdm7 (USGS, HUC 10300101) looks like a bad case. The matching feature_ids in the flow file (highlighted in yellow) are all on the mainstem, but the domain (black) is on the tributary and none of the tributary segments have flow.

[mluck]

I used stream order to differentiate the tributary levelpath from the mainstem levelpath, but I found at least one case where there were two levelpaths of the same order in the domain. In case this happens, the flow is attributed to the longer levelpath (length in the domain) and not to the shorter levelpath. This agrees with the validation raster (gett2, HUC 12070205):

[RobHanna-NOAA]

I need more information about deployment. I think it needs a full scale run to get new data, although I think Ryan might have this already completed. If that was time, I need time to deploy to all enviros prior to merge. Do I have it right that we create a new set of benchmark data we need for Alpha testing? Same.. I will need time to get it deployed. Do you it to be deployed ahead of any particular build? I am worried about too much coming in one build. Can someone clarify deployment for me?

[mluck]

This will generate an updated set of AHPS (NWS and USGS) benchmark flow files based on the current benchmark flow files. For testing, the outputs are all saved to a single folder, but on the actual (i.e., post-testing) run, do NOT set the output option -o which will force the new benchmark flow files to overwrite the existing benchmark flow files (after a copy of the originals are saved with a .bak extension in the same folder).

Then the new benchmark flow files will need replace the old benchmark flow files (and possibly also copy over the original .bak backup copies as well) the enviros prior to alpha testing.

[RyanSpies-NOAA]

Latest version of the alpha test metrics results look good (improved scores across the board).

NWS aggregate metrics (current vs. new eval flows):

USGS aggregate metrics (current vs. new eval flows):

[CarsonPruitt-NOAA]

Great news! I wasn't expecting to see any increased overprediction as shown in the Action stages. I'm guessing that's coming from the previously-labeled BAD_SITES that we added upstream flows to?

[RyanSpies-NOAA]

CBST2 (12090301) now shows a much improved agreement raster which can be attributed to the reduction in FP scores for the tributary domain:

[RyanSpies-NOAA]

Regarding the increased false alarm values (especially for the USGS sites) - it appears there are a couple of factors at play. I think @CarsonPruitt-NOAA is correct that including some of the previous "bad sites" is a key contributor. Here is NMSO1 as an example:
Previous flow file was not providing flow for the reaches within the evaluation domain (purple flow lines show where the flow file has values) which yielded a lot of false negative scores:

Modified flow file is now providing flows for the appropriate reaches (green reaches) and there is a sizeable increase in the false positive scores:

[mluck]

AHPS data for Alaska are also being processed in #1130.

[RyanSpies-NOAA]

Generated new flow files, successfully ran synthesize test case, and produced eval plots using existing 4.4.13.0 FIM outputs.

Locations that need to receive updated benchmark flow files (@RobHanna-NOAA):

• EFS
• Dev1
• ESIP S3
• ras2fim S3

[RobHanna-NOAA]

Copy the efs.. /usgs_test_cases/validation_data_usgs/ to all enviros, correct?

[mluck]

@RobHanna-NOAA This will update both usgs_test_cases/validation_data_usgs/ and nws_test_cases/validation_data_nws/

[RobHanna-NOAA]

Forgot to update. The data has been pushed to all enviros including ras2fim and ras2fim-dev