PSU-CSAR / vb-bagis

Source code for BAGIS V3 ESRI Add-In
0 stars 1 forks source link

ArcGIS AOI Creation #66

Closed jdduh closed 4 years ago

jdduh commented 5 years ago

Got this error message when creating an AOI from a BASIN.

image

The process was able to continue. This message shows up at the end. image

Here is the catalog view of the AOI. image

jdduh commented 5 years ago

When using the AOI in the analysis, I got this error message in 4. Generate Zones. image

The AOI does contain SNOTEL and snow course sites. It seems that the clipping of the webservices was unsuccessful. The pink dots are SNOTEL sites, blue are snow course sites. image

lbross commented 5 years ago

Can you post the files for 'testbasin' to Basins? I have not gotten these errors with the basin I am working with. Also, please confirm that you are using the default BAGIS settings, including the webservices.

Also, what version of AGS and what version add-in are you using?

jdduh commented 5 years ago

ftp://basins.geog.pdx.edu/BAGIS/BAGIS_aois/testbasin.zip This is a basin containing an AOI.

I used the Default BAGIS settings on ArcGIS 10.5.1 and got the addin from the ftp server. The library version is L039.

image

lbross commented 5 years ago

This is a handful. I have confirmed this is not a problem under AGS 10.2.2. First I thought the problem was the extra slashes in the path to aoibagis. It took me a while to track them down because multiple methods were updating the global variable 'AOIFolderBase' that should not have been. An argument against using global variables! But it still failed. The problem IS the snap raster path: $aoiName\aoi.gdb\aoibagis. When creating a AOI from basin this layer is originally created at the root of the AOI because there is a bug with the watershed tool where it can't write the output to a file geodatabase. We move this file into the correct location after doing all of the clipping. Thus you saw it there after the AOI creation had completed with errors. One option is to relocate the layer earlier in the AOI creation process but I hesitate to do that because I'm not sure what other processes might be using it at its current location. The easiest solution would be to use a different snap raster, like maybe filled_dem? We know that it will be in the surfaces.gdb by the time we need it. It is scary that this didn't fail under AGS 10.2.2. I can only assume that the 10.2.2 geodatabase tool (ExtractValuesToPoints) wasn't really using the snap raster layer. Also open to other suggestions if you have them?

lbross commented 5 years ago

I thought of a solution for this last night. I will make a copy of aoibagis, after it is created, in its final location where where the ExtractValuesToPoints tool is looking for it. I will continue to delete the original layer at the same place I always have so any other processes that might be using it would still have access to it. The end result should be the same, except that the ExtractValuesToPoints too should work!

jdduh commented 5 years ago

Sounds good. Thanks!

lbross commented 5 years ago

Once again, this was more challenging than I expected. With my original approach, I kept getting a locked file error when trying to delete the aoibagis layer at the aoi root at the end of creating an AOI. Instead, I copied the layer to the aoi.gdb right after creating it, deleted it, and fixed the follow-on references to use the aoi.gdb version. It works for me on both AGS 10.2.2 and AGS 10.5. (I am trying to keep the code the same as much as possible). Please download a new version of the AOI from Basins at your convenience. I didn't change the version number but the file date should be 2 November.

jdduh commented 5 years ago

It seems the bug is fixed. However, there are several issues.

  1. Initial error when the addin was first installed. This probably is an ArcGIS bug. The problem disappears after a restart of ArcMap.

    bagis_aoi tool error
  2. The snow course number is not correctly captured in the generate maps parameter file and GUI. The parameter file is attached. There is 1 snow course site in the AOI, but the analysis reported 0. Is it related to the elevation of the SC?

  3. BAGIS left a bunch of temporary rasters in the scratch workspace (i.e., in the AOI). Please find a way to clear them up. ArcCatalog doesn't see them as valid rasters. Them must be the remnant folders after the temp rasters were deleted.

image

map_parameters.txt

lbross commented 5 years ago

I am looking into the issue with the missing snow course number. This is also showing up under 10.2.2.

The other two issues are, I believe, related to the use of the Geoanalyst tools when creating an AOI. They seem to become more unstable with each revision of AGS. When I was troubleshooting the earlier error with ExtractValuesToPoints, these tools threw an exception about 1 out of every 5 times I created an AOI. I needed to restart ArcMap and try again. These classes are also bad at managing their temporary files. I don't see the temporary rasters in the AOI when the AOI is created successfully. But I do see them when an exception occurs. My guess is that the files are locked so ArcMap can't clean up after itself. Even if I tried to delete them through another process during the same session, I likely couldn't because they would be locked.

My recommendation is to try rewriting the AOI boundary creation code to use the Geoprocessor instead. It will be slower, but hopefully more stable. This will take some development time, so please let me know if you want me to try this:

  1. pHydrologyOp.SnapPourPoint -> SnapPourPoint
  2. pExtractOp.Attribute(pRasDes) -> ExtractByAttributes (where value > -1)
  3. pConversionOp.RasterDataToPointFeatureData -> RasterToPoint
  4. pHydrologyOp.Watershed -> Watershed
lbross commented 5 years ago

Just found the bug with the snow course sites. It was a silly coding error and does apply to all versions. I can do a new release when you decide if we should rewrite some of the AOI creation code.

jdduh commented 5 years ago

Yes, please rewrite the code to use Geoprocessor. Please build a new release when this is done. Thanks!

lbross commented 5 years ago

Question: There are 2 paths when creating the initial AOI boundary. The first path, outlined in the previous comment, is followed if the "Automatically snap pourpoint" checkbox is ENABLED. The second path just calls the Watershed tool if the checkbox is DISABLED. Should we be looking at whether the checkbox is CHECKED instead? This code was written before I started working on BAGIS so I'm not sure of the original intent.

lbross commented 5 years ago

@jdduh: Please see my question above. I've found a couple of other bugs with the current version of BAGIS, so would like to get a new release in circulation soon.

I have refactored the AOI boundary creation code to use the GP instead of Geoanalyst ArcObjects. This seems to be more stable and doesn't create the temporary files. However, I found that the initial step on the Maps form creates multiple temp files in the AOI directory. I wrote a method to delete any rasters from the AOI root to clean these out after that step runs. To my knowledge we aren't storing any rasters at the AOI root. I did find a couple of vectors, like the aoi_streams.shp in that folder but these aren't rasters and aren't touched.

I also found a bug with clipping the snotel/sc layers in the create AOI from shape file tool and fixed it. And I found that the step progressor didn't have enough steps in it. Have fixed this too. Although create AOI from shapefile still uses Geoanalyst ArcObjects, it isn't generating any temp files at the AOI root so I didn't make any changes to this code.

jdduh commented 5 years ago

"Should we be looking at whether the checkbox is CHECKED instead?" Do you mean the current code doesn't verify if the checkbox is checked or not? Please update the code to branch out the process correctly.

lbross commented 5 years ago

It's confusing. It checks for CHECKED when determining the pourpoint layer name: unsnappedpp or pourpoint. But when it actually does the processing, it checks for ENABLED when deciding whether or not to run through the longer path before creating the watershed. I will change this and publish a new add-in. It makes more sense to me.