massbays-tech / MassWateR

R package for working with Massachusetts surface water quality data
https://massbays-tech.github.io/MassWateR
Creative Commons Zero v1.0 Universal
11 stars 3 forks source link

Value Range for Lab Spike % Recovery #60

Closed ben-wetherill closed 1 year ago

ben-wetherill commented 1 year ago

This is a minor and rare situation, and there are ways to work around it, but the package is not currently handling the value range for lab spikes correctly. Would this be easy to handle?

For the result unit of measure, we have one special exception: Lab Spikes may be entered with a percent unit of measure "%" or "% recovery" instead of the base unit of measure. If the value ranges are defined so that values around 100 have a different DQO, then the wrong DQO might be used by the package. In the attached files, both lab spikes for Nitrite should be misses, but only one is flagged. I think the logic should be that if lab spikes are entered with the "%" or "% recovery" unit of measure, then the lab spike should be evaluated against the upper value range, and if the upper value range is not a % DQO, then tabMWRacc() should return an error.

Sample_DQOAccuracy_testing.xlsx Sample_Results_testspikes.xlsx Sample_DQOFreqComp_testingspikes.xlsx

fawda123 commented 1 year ago

@ben-wetherill this makes sense and should now be handled correctly for the accuracy checks. There's also a new error that's returned if the upper value range is not a percent value in the DQO file for results with units as percentages.

I also noticed for the lab spikes/instrument checks that the comparison of any percent recovery value to a percentage DQO was not using the absolute value. The example you provided was not being flagged as a miss because the % percent recovery was -18% (100% - 82%) and -18% was less than the <=15% DQO. So, I changed the way the code handles the comparison by using absolute value for the percent recovery. This didn't seem to change any of the other results, except for the one nitrite check that is now correctly marked as a miss.

ben-wetherill commented 1 year ago

I'm very surprised to hear that % recovery was not evaluated as an absolute value because that is a fundamental piece of the % recovery logic. I'm certain I would have tested that, but you are right that the current production version of MassWateR does not flag negative % recovery exceedances. Is it possible that this changed somewhere along the way?

The fixes look good mostly, but I did find one issue. It is also related to the absolute value concept: In the files I provided, if you change the Nitrite lab spike result value from 460 to 340, tabMWRacc() does not flag it as a miss. It should because the absolute value of the difference is 60, which is greater than 50.

So, it turns out that spike/check accuracy was not evaluating absolute value for anything (% or normal units of measure). I am 100% certain that I tested this before, so something must have changed.

fawda123 commented 1 year ago

@ben-wetherill I fixed the issue for the lab spike results not evaluating the absolute value. I agree it's odd that we didn't catch that during initial testing.

ben-wetherill commented 1 year ago

I think this looks good now. Thank you.