search

NOAA-OCM / QNSPECT

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

Comparison Percent Calculation Incorrect? #108

Closed ar-siddiqui closed 2 years ago

ar-siddiqui commented 2 years ago

Latest QNSPECT version

Similar issues do not exist

What is the bug or the crash?

Lets say we have raster A = [[5,1]] and B = [[1,2]]

When we calculate percent difference here expression="100 * ((A - B) / A)"

Our result will be [[80%, -100%]]. How do we interpret this ?

  1. A is 80% of B, and A is -100% of B?
  2. B is 80% smaller than A, B is -100% smaller than A, which means B is 100% greater than A?

How to interpret results?

Steps to reproduce the issue

N/A

Screenshots and Attachements

No response

Versions

3.22.4

Additional context

No response

NSPECT commented 2 years ago

Good catch, the formula should be 100 * (B - A)/ A which is the change from A to B:

The results using your numbers would then be: [[-20%, 100%]], a decrease to 20% for the first element, and a increase of 100% for the second.

NSPECT commented 2 years ago

Oops, [[-80%, 100%]]. math is hard.

ar-siddiqui commented 2 years ago

Since, our Results Group name is A vs B I think what we have might already be correct. A is 80% greater than B, A is 100% smaller than B.

vs

If we change the formula it will be B is 80% less than A and B is 100% more than A

What do you think will be better considering our group name A vs B? I think it is just a way of interpretation but thinking about it more I like the first one.

DaveEslinger commented 2 years ago

Sure, A vs B is ok, but then the equation becomes 100 * (A - B)/B. That way, if there if A DECREASED, the percentage is negative; if A increased, the percentage is positive.

I would like to stick with that usage: negative means a decrease, that is what we've trained users to expect. Think of it as the change from some original state to a new state. The percent change would be: 100 * (New - Original) / Original, and our naming convention would be New vs Original.

In the current code, I just think the denominator is wrong; it should be B.

ar-siddiqui commented 2 years ago

Ok 100 * (A - B)/B makes much more sense. Glad we caught this. Is this the same what OpenNSPECT had or is it different?

DaveEslinger commented 2 years ago

It should be the same.