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: Renaming of benchmark data and associated variables #290

Merged
merged 25 commits into from
Mar 2, 2021

Conversation

BradfordBates-NOAA
Copy link
Member

@BradfordBates-NOAA BradfordBates-NOAA commented Mar 2, 2021

The benchmark inundation rasters were named "depth" instead of "extent", and NWS and USGS domain shapefiles were named "extent" instead of "domain". This branch addresses those issues in the test_cases directory and in run_test_case.py as well as run_test_case_calibration.py, although the latter will likely be deprecated.

Testing

To test, you can run:

python foss_fim/tools/run_test_case.py -r fim_3_0_4_4_ms_c_agg/12090301/ -b dev-rename-pr-nws -t 12090301_nws

and then run:

python foss_fim/tools/run_test_case.py -r fim_3_0_4_4_ms_c_agg/12090301/ -b dev-rename-pr-ble -t 12090301_ble

And confirm no Traceback errors and good-looking outputs.

@TrevorGrout-NOAA
Copy link
Contributor

In running the first command: python foss_fim/tools/run_test_case.py -r fim_3_0_4_4_ms_c_agg/12090301/ -b dev-rename-pr-nws -t 12090301_nws
A warning is produced when evaluating cbst2 for action magnitude:

RuntimeWarning: invalid value encountered in double_s                        calars
  MCC = (TP*TN - FP*FN)/ np.sqrt((TP+FP)*(TP+FN)*(TN+FP)*(TN+FN))

Also it appears to be iterating multiple times (minor/moderate/major), for example see screenshots:
image

In running the 2nd command: `python foss_fim/tools/run_test_case.py -r fim_3_0_4_4_ms_c_agg/12090301/ -b dev-rename-pr-ble -t 12090301_ble'

This appears to run without error.

I will verify look at outputs shortly but wanted to get this out there in case it needed addressing or maybe I am mis-interpreting the output.

@BradfordBates-NOAA
Copy link
Member Author

That warning often happens when a math operation is not possible, perhaps because one of the contingency categories had a zero pixel count. It has been happening for a while and should be unrelated to these changes.

@TrevorGrout-NOAA
Copy link
Contributor

The ble outputs appear to be in place and the visually look appropriate. I verified that the huc wide CSI are consistent from pr-rename-pr-ble vs fim_3_0_4_4_ms_c for both 100 and 500yr.

The nws outputs appear to be in place and visually look appropriate. I verifed that for major category BRTT2 has consistent CSI from dev-rename-pr-nws.
Recommend investigating artifact issues related to messages printed to screen and domain extents for instances where no flow files are supplied (ahps) as future tickets.
I approve this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[5pt] Change "depth" to "extent" in benchmark rasters and update eval scripts accordingly
2 participants