[1pt] PR: Fix eval_plots bug where a benchmark source has no data

Open RobHanna-NOAA opened this issue 5 months ago • 0 comments

During some testing of the new PR 1620 :Redesign Calibration workflow, a branch error occurred. During a alpha test against a small huc sample set with a branch error, it exposed a bug in eval_plots.py. The bug is normally not seen as in order to see the error, you have to have a huc list what errors out on one HUC but no other HUCs in that test run with a valid benchmark source.

Note: This bug is not related to PR 1620 and has existed for a long time.

This PR should not be merged until 1620 is merged with dev.

eval_plots.py assume there would be some metrics data a given benchmark type for a given pipeline run and it could end up as an empty dataset.

Changes

tools
- eval_plots.py: Changes include:
  - Added more prints to help sort out progress and more context clues when something fails.
  - Adjusted a few variable names to be more initiative.
  - Added a bit of input validation code.
  - Added some inline validation code to ensure some datasets are not empty in key places.
  - Fixed a bug when the tool is being used for spatial data, creating the FIM Performance points and poly files. A previous merge accidently changed a key variable name which would have resulted in the two FIM Performance files never being created.
  - Add more doc strings.
- synthesize_test_cases.py: Added a few warning message and upgrade a bit of the wording on an error message.
- run_test_case.py: Found a bug where shutil.rmtree could fail with directory not empty during race conditions of the python GC. Could have been MP cleaning overlapping or subdirectories at the same times. Added the "ignore_error=True" tag to shutil.rmtree.
- probabilitic_inundation.py: Added the "ignore_error=True" tag to shutil.rmtree.

Closes 1695

Testing

If you want to see a before and after, you will need a small HUC run with two versions of eval_plots run against it. One version to eval_plots from our dev branch and the other from this branch.

The bug was that it will fail while running eval_plots as it doesn't know how to handle a empty dataset.

The pipeline run you could use is: HUCs: 07080103 07080208 12040101 17050114 12040103 12090301 19020302. It does not matter which branch you use to run pipeline, either dev or this branch.

This combination of HUCs means all five benchmark categories have at least one HUC related to that category.

However, while 07080208 does have ras2fim applicable data, as in the test_cases_ras2fim folder, the data will fail actual eval_plots processing. 07080208 when applied to ras2fim benchmark category will fail the masked_perc<97 test. This results in no valid remaining ras2fim valid HUCs to process after filtering.

Using the new branch, run synthesize_test_cases.py. It will work for both the pre fail and post fix eval_plots fix.

To see the error pre-fix if for eval_plots.py, run just eval_plots.py against the dev branch to see the following error:
To see the post fix, run it against this branch and you should see:

Deployment Plan (For FIM developers use)

Does the change impact inputs, docker or python packages?
- [ ] Yes
- [x] No (f no.. skip the rest of the Deployment Plan section)
If you are not a FIM dev team member: Please let us know what you need and we can help with it.
If you are a FIM Dev team member:
- Please work with the DevOps team and do not just go ahead and do it without some co-ordination.
- Copy where you can, assign where you can not, and it is your responsibility to ensure it is done. Please ensure it is completed before the PR is merged.
- Has new or updated python packages, PipFile, Pipefile.lock or Dockerfile changes? DevOps can help or take care of it if you want. Just need to know if it is required.
  - [ ] Yes
  - [ ] No
- Require new or adjusted data inputs? Does it have a way to version (folder or file dates)?
  - [ ] No
  - [ ] Yes
    - Require new pre-clip set or any other data reloads, such as DEMS, osm, etc. ie.. pre-requisite re-data upstream of your input changes.
      - [ ] Yes
      - [ ] No
    - Has the inputs been copied/exist in all five enviros:
      - [ ] FIM EFS
      - [ ] FIM S3
      - [ ] ESIP
      - [ ] Dev1
      - [ ] UCS2
Please use caution in removing older version unless it is at least two versions ago. Confirm with DevOps if cleanup might be involved.
If new or updated data sets, has the FIM code, including running fim_pipeline.sh, been updated and tested with the new/adjusted data? You can dev test against subsets if you like.
- [ ] Yes

Notes to DevOps Team or others:

Please add any notes that are helpful for us to make sure it is all done correctly. Do not put actual server names or full true paths, just shortcut paths like 'efs..../inputs/, or 'dev1....inputs', etc.

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.

[x] Informative and human-readable title, using the format: [_pt] PR: <description>
[x] Links are provided if this PR resolves an issue, or depends on another other PR
[x] 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)
[x] Changes are limited to a single goal (no scope creep)
[x] The feature branch you're submitting as a PR is up to date (merged) with the latest dev branch
[x] pre-commit hooks were run locally
[x] Any change in functionality is tested
[x] New functions are documented (with a description, list of inputs, and expected output)
[x] Placeholder code is flagged / future todos are captured in comments
[x] CHANGELOG updated with template version number, e.g. 4.x.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

Nov 19 '25 19:11 RobHanna-NOAA