NOAA-OCM / QNSPECT

QGIS Plugin for NOAA Nonpoint Source Pollution and Erosion Comparison Tool (NSPECT)
GNU General Public License v2.0
8 stars 5 forks source link

Add time units to output #92

Closed NSPECT closed 2 years ago

NSPECT commented 2 years ago

Addresses question raised by #92. This change checks the number of raining days. If it is 1, units are per event, if more than one, then it is assumed to be an annual estimate and units are per year.

The time units for erosion calculations are always per year. RUSLE is an annual model, so the time units do not change.

Output: image

Note there is still the issue with units not always loading correctly.

ar-siddiqui commented 2 years ago

To merge this PR, we need to update the following as well

ar-siddiqui commented 2 years ago

I liked the idea. My only worry is that our tool is already a bit complicated and this adds to the complication.

One idea to reduce complication is to make Number of Rainy Days in a Year variable optional and if a value is not provided we assume event based simulation. If a value is provided we assume annual simulation, we convey this through documentation.

DaveEslinger commented 2 years ago

Hi @ar-siddiqui, I'd prefer not to make the Rainy Days field an option, since using it is used in what is, far and away, the most common and most appropriate use of the tool. Making it optional requires a user to do some "extra" work, and complicates the user experience. The current method doesn't affect the user's work flow, and makes the code only slightly more complex (one if statement to assign a variable, and several instances of concatenating that variable onto labels), with all complexity handled out of the users view.

Note that I initially thought we should just hard-code in the yearly units, but conceded to my colleagues that we should allow for some variation for advanced users who might want to "game" the tool and use it for estimating output from a single event.

NSPECT commented 2 years ago

Text fields modified to account for variable labels. Note also that all instances of "rainy_days" were replaced with raining_days, which is the more usual term, at least for NSPECT users.