Expose Threshold between Estimated Active & Inactive Valley Bottom as Parameter and Change Default

[joewheaton]

We are displaying in "Estimated Valley Bottom" Geomorphic Units Active as likelihood 0.99 to 0.8 and inactive 0.8 to 0.68. From what I've seen, this is not a good estimate. I am speculating that 0.9 will be better as a rough-cut of active vs. inactive. As such, @philipbaileynar can we change VBET to:

• Expose this threshold between these two polygons as a parameter for CyberCastor CLI in VBET
• Change the default from 0.8 to 0.9

Rationale from #245

From #362 this changes to:

CategoryID	Likelihood	Label in Legend	Notes/Rationale	Old Threshold
1	>= 0.99	Estimated Active Channel	100
2	0.99 < Polygon >= 0.90	Estimated Active Floodplain	70
3	0.90 < Polygon >= 0.68	Inactive Floodplain	50
4	>= 0.68	Plausible Valley Bottom	50

[shelbysawyer]

@joewheaton Makes sense.. I noticed that the active was a little generous when cruising around through VBET. If 90% ends up being too generous for inactive is that necessarily a bad thing? Or would we work the numbers again?

[philipbaileynar]

@shelbysawyer can you please take the VBET project for 10190005 and threshold the combined evidence raster at 0.9 and then paste a screenshot here (with your best attempt to match Joe's symbology if possible).

In other words, lets avoid making time consuming and expensive changes to software merely on speculation. Let's do the cheap and easy task of producing one example manually and proving to ourselves that it's what we really think it is.

[MattReimer]

Might I suggest @philipbaileynar & @shelbysawyer let's wait for VBET 0.4.0 before we do this work so we can be sure we're using all the latest changes

[shelbysawyer]

@MattReimer Okay. I'm happy to play with this if it would a helpful visual, even if based on the outdated tool. Just let me know.

[MattReimer]

Sure. It's just my job to make sure everyone knows what version things are at 😄

I've updated the release notes with versions so we know we're using the current stuff
https://github.com/Riverscapes/riverscapes-tools/releases/tag/Sep2021_release

[philipbaileynar]

Further supporting the decision to rush in and change thresholds without first proving their value... remember that we use the vbet_80 and vbet_68 as layer names. Changing one or more of these breaks business logic and creates a ton of work. Better to confirm first that the change is of value.

Perhaps this also suggests that baking the threshold into the layer name is a bad idea...

[MattReimer]

Kelly and I just discussed this. Places where the name vbet_68 exists:

• The python tool py file (for vbet)
• The python metadata augmentor scripts (for brat,confinement, rvd etc)
• All relevant Cybercastor automation scripts (because we pass it in as a filename)
• Businesslogic XMLs

I'm open to any suggestions how we can do this in a generic way but I can't think of one because the layers need names and each threshold is a layer.

Maybe we could squash all the geometry onto a single layer and have the threshold be a field?

[philipbaileynar]

Maybe we could squash all the geometry onto a single layer and have the threshold be a field?

Don't like this. Makes it really hard for downstream tools to use subset of features.

[joewheaton]

@philipbaileynar let me know if you want to update this (I recall you jotting down notes on naming convention we agreed upon).

[philipbaileynar]

Closing this issue in favour of #428

#427 is also relevant.

[joewheaton]

What about exposing this as parameter @philipbaileynar ?

[KellyMWhitehead]

Move the threshold values to a vbet parameter. Next steps will be to add this to launch and automation scripts.

Defaults to 68% (Full vbet) and 90% (IA vbet)