Esri / military-tools-geoprocessing-toolbox

military-tools-geoprocessing-toolbox is a collection of models, scripts, and tools for use in ArcGIS for Desktop and ArcGIS Pro. This toolbox is one component that is a part of Military Tools.
Apache License 2.0
33 stars 14 forks source link

Find Local Peaks is failing on ArcGIS Pro 2.1 #345

Closed dfoll closed 6 years ago

dfoll commented 6 years ago

Current Behavior

When you run the Local Peaks tool on ArcGIS Pro 2.1 it is failing with ERROR000229 and says it cannot open the invertedSinks feature class which is part of the intermediate data of this tool. This was also happening for me with the released version of Military Tools (using the ArcGIS toolbox as opposed to the Python toolbox). So far some team members have been able to reproduce this and some have not. Did not experience this in ArcMap 10.4.1 when using the same build.

Versions this is been seen on

ArcMap 10.3.1 good for @ACueva ArcMap 10.4.1 - good for @dfoll ArcMap 10.5.1 - good for @kgonzago
ArcMap 10.6.1 - fails for @ACueva (in comments) ArcGIS Pro 2.1 - fails for @dfoll works for @BobBooth (feature sets wrok for @dfoll ) ArcGIS Pro 2.2 fails for @topowright

Steps to Reproduce

  1. Input Area: D:\TestData\MilitaryToolsTestData.gdb\AreaofInterest
  2. Input Surface: D:\TestData\MilitaryToolsTestData.gdb\ElevationUTM_Zone10
  3. C:\Users\dere5261\Documents\ArcGIS\Projects\NewLocalPeaksTesting\NewLocalPeaksTesting.gdb\localpeaks

Your Environment

ArcGIS Pro 2.1 MT build 43

image

dfoll commented 6 years ago

@lfunkhouser to evaluate priority and decide what sprint to bring it into

ACueva commented 6 years ago

Experiencing this same behavior in 10.6:

Executing: FindLocalPeaks AreaofInterest 10 ElevationUTM_Zone10 C:\Users\avel5840\Documents\ArcGIS\Default.gdb\ElevationUTM_Zone10_FindLoca
Start Time: Mon Jun 18 15:57:35 2018
Running script FindLocalPeaks...
Using WGS_1984_UTM_Zone_10N for analysis.
Clipping ElevationUTM_Zone10 to area tempAreaFeatures...
Inverting clipped surface...
_getRasterMinMax min=-7.0, max=1123.0
Finding inverted sinks...
Executing: GetCount \Documents\ArcGIS\Default.gdb\invertedSinks
Start Time: Mon Jun 18 15:57:48 2018
ERROR 000229: Cannot open \Documents\ArcGIS\Default.gdb\invertedSinks
Failed to execute (GetCount).
Failed at Mon Jun 18 15:57:48 2018 (Elapsed Time: 0.01 seconds)
Removing intermediate datasets...
Removing: \Documents\ArcGIS\Default.gdb\tempAreaFeatures
Removing: \Documents\ArcGIS\Default.gdb\clipSurface
Removing: \Documents\ArcGIS\Default.gdb\invertedSinks
Done
Completed script FindLocalPeaks...
Failed to execute (FindLocalPeaks).
Failed at Mon Jun 18 15:57:51 2018 (Elapsed Time: 15.69 seconds)
kgonzago commented 6 years ago

@dfoll @BobBooth @topowright I was able to solve the issue of GetCount() throwing an error. But now I get an error when calling the RasterToPolygon_conversion(). My code is checked into this branch. Has anyone seen this error when converting rasters to polygons: image

topowright-zz commented 6 years ago

The challenge that I understand with this is that the data is not being written to the default database; this is the reason why it cannot access the table. @kgonzago can you explain what you did to fix the issue with the inverted sinks writing to the database?

kgonzago commented 6 years ago

@topowright invertedSinks has always been written to the project database. The reason GetCount() didn't work was that function was being called on invertedSinks which is a raster. That raster needed to be converted to a polygon feature class in order for the GetCount() to work. But the new issue is RasterToPolygon() doesn't work on invertedSinks (see my comment)

kgonzago commented 6 years ago

Executing Raster To Polygon tool manually worked but once running that tool in the Find Local Peaks script throws an exception.

mfunk commented 6 years ago

Getting close on this one. The problem goes back to a fix from a commit a year ago: https://github.com/Esri/military-tools-geoprocessing-toolbox/commit/170e675d130bd2d6b5cfd8af176dce3e84b72698 ... where GetCount was used to check the number of sinks found in the inverted area. Unfortunately the GetCount tool requires that an attribute table be present on the input dataset, in this case the 'invertedSinks' must have a VAT... which it sometimes will and sometimes will not. If the VAT isn't there, GetCount fails.

So i'm working on adding a VAT with BuildRasterAttributeTable before the GetCount, however BRAT fails inside the script with an error...

Executing: BuildRasterAttributeTable D:\Workspace\Test\2018-06-22_MT_FindLocalPeaks\scratch.gdb\invertedSinks Overwrite
Start Time: Fri Jun 22 13:55:34 2018
ERROR 000049: Failed to build attribute table
The table was not found.
The table was not found. [fras_blk_Sink_flowDir1]
The table was not found. [VAT_invertedSinks]
Failed to execute (BuildRasterAttributeTable).
Failed at Fri Jun 22 13:55:34 2018 (Elapsed Time: 0.05 seconds)
topowright-zz commented 6 years ago

We do not need to work on this. We are using help from Matt Funk to address this issue being that he found an error with a tool that needs to create a table that is used by get attribute.

mfunk commented 6 years ago

@topowright @lfunkhouser

Changing the 'invertedSinks' workspace to '_inmemory' seems to get around the problem with BuildRasterAttributeTable.

        saSink = sa.Sink(saFlowDirection)
        invertedSinks = os.path.join("in_memory", "invertedSinks")
        saSink.save(invertedSinks)
        deleteme.append(invertedSinks)

        #need to make sure there is a VAT for GetCount
        arcpy.BuildRasterAttributeTable_management(invertedSinks, "Overwrite")

if I run the tool...

Executing: FindLocalPeaks AreaofInterest 10 ElevationUTM_Zone10 D:\Workspace\Test\2018-06-22_MT_FindLocalPeaks\scratch.gdb\asdfasdf
Start Time: Fri Jun 22 15:53:55 2018
Running script FindLocalPeaks...
Using WGS_1984_UTM_Zone_10N for analysis.
Clipping ElevationUTM_Zone10 to area tempAreaFeatures...
Inverting clipped surface...
_getRasterMinMax min=-7.0, max=1123.0
Finding inverted sinks...
Found 10518 sinks
10518 sinks found
Converting sink values to polygon features...
Extracting elevation values from ElevationUTM_Zone10...
Found 10 peaks of with elevations [1081, 1080, 1075, 1063, 1061, 1057, 1051]
Removing intermediate datasets...
Removing: D:\Workspace\Test\2018-06-22_MT_FindLocalPeaks\New File Geodatabase.gdb\tempAreaFeatures
Removing: D:\Workspace\Test\2018-06-22_MT_FindLocalPeaks\New File Geodatabase.gdb\clipSurface
Removing: D:\Workspace\Test\2018-06-22_MT_FindLocalPeaks\New File Geodatabase.gdb\invertSurface
Removing: D:\Workspace\Test\2018-06-22_MT_FindLocalPeaks\New File Geodatabase.gdb\flowDir
Removing: in_memory\invertedSinks
Removing: D:\Workspace\Test\2018-06-22_MT_FindLocalPeaks\New File Geodatabase.gdb\sinkPolys
Removing: D:\Workspace\Test\2018-06-22_MT_FindLocalPeaks\New File Geodatabase.gdb\pointSinks
Removing: D:\Workspace\Test\2018-06-22_MT_FindLocalPeaks\New File Geodatabase.gdb\sinkValues
Done
Completed script FindLocalPeaks...
Succeeded at Fri Jun 22 15:54:25 2018 (Elapsed Time: 29.83 seconds)
csmoore commented 6 years ago

Just checking if this is ready to work - are we making the change suggested above or waiting on a PR with this change?

csmoore commented 6 years ago

Moving in-progress to implement/test suggested workaround above

csmoore commented 6 years ago

I tested this both pre and post suggested workaround and did see the problem pre-workaround in Pro 2.1 and ArcMap 10.6.1.

After applying the workaround from PR #349 things seemed to work (screenshots below).

Additional Note: @mfunk - was not sure if we should add this workaround or investigate why/how the behavior or SpatialAnalyst changed - the workaround PR #349 is available if needed.

Pro (Note: I did notice the labels don't show up here - not sure if that is expected behavior): image

ArcMap: image

topowright-zz commented 6 years ago

This has now been fixed with the workaround based on the core tool not writing a table everytime.

BobBooth commented 6 years ago

Removed tags for Dev and QC Verify (Checklist) - @topowright and I tested together.