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

[5pt] Set up CatFIM to run in Alaska #1117

Open
EmilyDeardorff opened this issue Apr 11, 2024 · 6 comments · May be fixed by #1285
Open

[5pt] Set up CatFIM to run in Alaska #1117

EmilyDeardorff opened this issue Apr 11, 2024 · 6 comments · May be fixed by #1285
Assignees
Labels
CatFIM NWS Flood Categorical HAND FIM

Comments

@EmilyDeardorff
Copy link
Contributor

Update the CatFIM code and data inputs as needed so that the stage-based and flow-based CatFIM can run in the newly incorporated southern Alaska HUCs.

@EmilyDeardorff EmilyDeardorff added the CatFIM NWS Flood Categorical HAND FIM label Apr 11, 2024
@EmilyDeardorff EmilyDeardorff self-assigned this Apr 11, 2024
@EmilyDeardorff
Copy link
Contributor Author

In order for the CatFIM code to run on the Alaska HUCs, the FIM output needs to have the usgs_elev_table.csv object in the outputs folder. This means that the USGS gage inputs need to be updated to contain the USGS gages in Alaska.

@RobHanna-NOAA
Copy link
Contributor

This issue was attached to PR 1165 which started as a branch and PR to add Alaska. However, we had lots of trouble getting CatFIM to even run, so PR 1165 was taken over a PR to get it running again. This Issue remains open for a separate branch and PR in the near future.

@EmilyDeardorff
Copy link
Contributor Author

Flow-based:

I've just re-instated the CatFIM changes and run an example. It looks like we are close, however the CatFIM polygons have an incorrect CRS (see image).

I think a main priority will be to get the geopackage thing fixed in general, because otherwise it’s gonna be more difficult to check my work AND I have a feeling that error is occurring in a similar place to where the Alaska CRS error is occurring.

The flow-based CatFIM sites layer turned out fine with no CRS errors.

image
image

@RobHanna-NOAA
Copy link
Contributor

Awesome (almost working again) and Yes, I have seen problems with crs control throughout the product. In the end, we want it all to finish as one crs (3857 ? ). The problem with the gpkg is also due to the crs but it is only a 2 line addition to fix that but it will help tremendously. However, I recommend you and I meet up to talk about crs management in the product, aka.. when do we re-project and when. There is more to it then meets the eye, I think. Maybe not, but it is worth us double checking. :)

@EmilyDeardorff
Copy link
Contributor Author

EmilyDeardorff commented Sep 23, 2024

With most recent updates, stage-based CatFIM is creating weird boxes alongside the inundation polygons

Each inundation polygon has a corresponding block. The block is exactly large enough to fit the wbd08 polygon for the HUC.

image
image
image

it seems to be something I’ve added recently (or something rob has added recently), so I should toggle the changes to see if I can trace where this issue is coming from. it’s weird that it’s only stage-based… that could be a good clue. I fear it is something to do with the vertical datum… although I don’t know what it would be.

Processes that could be affecting it:

  • tif→geopackage processing
    • happens in: post_process_cat_fim_for_viz()(stage AND flow-based)
    • it would be weird if it was an issue with this code because the issue is happening just for stage-based but this function is run for stage- AND flow-based CatFIM
    • could test this by pulling the code into a temp file and then tweaking tests
  • tif creation
    • happens in: produce_stage_based_catfim_tifs()(just stage-based)
  • vertical datum adjustment
    • happens in: __adjust_datum_ft()

Debugging checklist (in progress):

  • not clearing the folder before re-running catfim
    → tested, not the cause
  • metadata pre-pull
    → tested, not the cause
  • crs=src.crs new line of code
    → tested without, didn’t make the issue go away
  • 2024-09-20 21:26:59 | WARNING || Traceback (most recent call last): File "/foss_fim/tools/generate_categorical_fim.py", line 982, in __adj_dem_evalation_val lid_usgs_elev = acceptable_usgs_elev_df.loc[ IndexError: index 0 is out of bounds for axis 0 with size 0
    → doesn’t correspond with the LIDs that have the issue
  • Alaska datasets causing projection confusion… causing weirdness with the vertical datum
    → could be happening in __adjust_datum_ft()
  • Alaska datasets causing projection confusion… causing masking weirdness
  • test dev branch to make sure that stage-based isn’t doing this weird stuff there as well
  • test flow-based of this branch one more time, to confirm that flow-based is not doing the weird stuff
  • test if it’s the tif weirdness by comparing the dev benchmark run with the weird new stage-based run outputs
    → running benchmark rn
  • (from Carson) make sure we are carrying forward the nodata value in the raster, because it seems like it could be that a nodata value is being turned into a number and processed acoordingly
  • (from Matt) our change to using Fiona could be causing files that weren't previously changed to be saved, which could contribute to blank tifs being turned merged into the geopackages as well

@RobHanna-NOAA
Copy link
Contributor

With Emily and I comparing notes and past check ins we discovered that the problem was created exactly when we upgraded CatFIM to work with the latest docker image

When we did the full last 4.5.2.11 run, we knew the code didn't work on again the latest docker image of fim_4:dev_20240724_b662494 and we would have to come back to it. We had to run it on fim_4:dev_20240823_6321468

The first thing I did on when I created my new 2.1 branch (dev-catfim-v2-1), was fix the docker image and checked it in.. I was watching the "sties" csv and it's points in QGIS and it was fine. But I did not open the library.csv with the polys. and sure enough, it was wrong.

So. Alaska is currently have the same carried over problem from the very first docker fix.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CatFIM NWS Flood Categorical HAND FIM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants