Riverscapes / pyBRAT

pyBRAT - Beaver Restoration Assessment Tool (Python)
http://brat.riverscapes.xyz
GNU General Public License v3.0
10 stars 10 forks source link

Management Unsuitable or Limited...bug) #199

Closed MatthewMeier closed 6 years ago

MatthewMeier commented 6 years ago

@banderson1618 looking over the outputs from BRAT it appears that there is an error in the code for indicating whether a segment is slope limited for the oPBC_UD field. It appears that many of the segments that are greater than 23% are being classified as being hydrologically limited instead of slope limited. In speaking with @joewheaton and @wally-mac there should be more segments that are slope limited than what we are seeing. In reference to the code under the Conservation_Restoration.py there seems to be an order of operations error occurring between line 48-68 specifically looking at line 52 dealing with slope limited segments and possibly line 62 as well with vegetation limited. It seemed to us that if it were slope limited it would be classified as that and then if it were less than 23% grade to be classified as hydro limited. Could you look into this. @joewheaton let me know if I misunderstood.

bangen commented 6 years ago

To clarify there isn’t an error with the code. I coded this up as Joe had outlined (see issue #178). If the logic needs to be tweaked you should work closely with either Joe and/or Wally.

joewheaton commented 6 years ago

@bangen (should not be working ;).... Over achiever! However, she's sort of correct. More specifically:

In #161, I asked for in the Naturally Unsuitable: Slope Limited class we have:

  • oCC_EX is None &
  • oVC_EX is occasional or higher &
  • iGeoSlope above whatever thereshold we change it in FIS (I think this is that above 23% range, but we would just make it iGeoSlope>23 here

and @bangen coded that in lines 48-68 of ConservationRestoration.py as:


 # 'oPBRC_UD'
    with arcpy.da.UpdateCursor(out_network, fields) as cursor:
        for row in cursor:
            # 'oCC_EX' None
            if row[6] <= 0:
                # 'oVC_EX' Occasional, Frequent, or Pervasive
                if row[4] >= 1:
                    # 'iGeo_Slope' >= 23%
                    if row[7] >= 23:
                        row[1] = 'Slope Limited'
                    # 'iGeo_Slope' < 23%
                    else:
                        row[1] = 'Hydrologically Limited'
                # oVC_PT' None
                elif row[3] <= 0:
                    row[1] = 'Vegetation Limited'
                else:
                    row[1] = 'Anthropogenically Limited'
            else:
                row[1] = 'NA'
            cursor.updateRow(row)

If I'm reading that correctly, line 56 (i.e. if row[7] >= 23:) should be finding all those steep areas. However, it is not. So that suggest to me that either a) its not getting in there ever (i.e. that row[7] or iGeoSlope, is never meeting that condition! i.e. slope is never > 23) or b) it is getting over written later. I don't think b) is happening. However, just as a I write this, I'm thinking that a) is more likely and maybe even though iGeoSlope is supposedly percent slope, maybe its actually in decimal.

I just checked: slope Notice max slope is 0.58! So @bangen it actually is a minor bug. This would be better @banderson1618 if exposed anything that is a threshold like this as a parameter in tool (i.e. in dialogs of tools in toolbox) instead of leaving it hardcoded.

@banderson1618 I will fix this real quick. Can you package it up into another release for @MatthewMeier to test on? Thanks!