forestgeo/HarvardMortality

should "DWR" be "Dead with resprout" ?

Closed this issue · 19 comments

@rudeboybert

I am working on setting up CI for Ordway Swisher Forest Dynamics Plot (OSFDP), reusing your script (Harvard) rather than mine (SCBI).

I just noticed the script calls for mort$DWR in a few different lines (see one example below), but, unless I miss something, I believe that column is actually Dead with resprout. I am changing it for OSFDP but I'll let you double check that it is needed for Harvard too.

If you don't have the time to do check+fix, let me know and I can look at it more closely for Harvard and do the fix myself.

https://github.com/SCBI-ForestGEO/HarvardMortality/blob/729ea6a991f425471ae78580840d641359e144b4/R_scripts/Generate_reports.R#L211

same question with column Canker; swelling, deformity, the script calls for canker,swelling,deformity instead

And same with Rotting trunk, the script calls for rotting main stem instead.

I think these are issues are due to inconsistencies between column names with SCBI...

Oooof good catch, I'll fix these in a new PR

Drat x3. To summarize what Valentine pointed out, the following mismatched codes between SCBI and HF eluded me when cloning the SCBI repo to HF

SCBI HF
DWR Dead with resprout
canker,swelling,deformity Canker; swelling, deformity
rotting main stem Rotting trunk

I implemented the fix #32. If you look at the resulting file diffs, new auto fix and (unfortunately) field fix errors are identified:

> require_field_fix_error_file$error_name %>% table
   canker_but_no_level       rot_but_no_level status_A_but_unhealthy 
                    18                    156                   9075 
> will_auto_fix_error_file$error_name %>% table()
unhealthy_but_wrong_status 
                      9075 

Questions:

  • @mageeluke Would you be able to fix the auto fix errors?
  • @djj4tree At this point I'm guessing there is nothing we can do about the field fix errors. For sake of the error map below, how about I disable the above three field fix error checks for this year?

Sorry about this folks, this is on me :(
map_of_error_and_warnings

Ouch! What a bummer. We did write those tests awfully fast in the push to get this running ahead of the census start, so don't beat yourself up, @rudeboybert . For the sake of the paper, and probably this map, these were not included in the set of "detectable errors" at the time the census was done, and therefore should not be used to evaluate the system (which caught a ton of error that would not have otherwise been caught).

@rudeboybert, I'm looking at the HF final data, and can't seem to see where the "status_A_but_unhealthy" are arising from. When I filter the data, only three records show status 2021 "A" and also contain an FAD.

I should be able to resolve most of the rot_but_no_level and canker_but_no_level through the photos.

This how I think we will prevent this to happen in the future:

  • push for a unified code that would apply to all sites (R package)
  • the code will be using an auxiliary file (that will belong to the package) that maps the column names of each site to a unified set of column name. @mageeluke and I started to work on a file like this here (work in progress).
  • add code in the scripts to avoid those "silent errors". I share a big part of the responsibility here, as the original code is mine and it turns out that it "works" without error when some non-existing columns are called... I can talk about this more with you @rudeboybert when we do the blameless post-mortem session you suggested (great idea!).

@mageeluke re: status_A_but_unhealthy the I'm a bit fried from teaching at the moment. I'll do a deep dive, as well as #3, by Monday mid-morning

I am now confused with the tests involving DWR (tests 10 and 11 here).

There are 3 options for DWR:

  • False
  • True
  • NA

What statuses are allowed for those 3 options?

Would it make more sens to change that column name to e.g. Resprout? I don't like that 'DWR' implies the tree is dead. The status column is the one telling us that.

FYI, @rudeboybert, current tests do not account for the NA case. I am changing them in the OSFDP to look at if value is "True" instead of "False", assuming an "NA" is more likely to be a "False" not entered.

Yes, per training at SERC with @djj4tree , DWR = NA is the same as DWR = FALSE.

DWR has been so rare that FALSE and NA are equivalent. @ValentineHerr I agree with making the test on value is 'True'

Drat x3. To summarize what Valentine pointed out, the following mismatched codes between SCBI and HF eluded me when cloning the SCBI repo to HF

SCBI HF
DWR Dead with resprout
canker,swelling,deformity Canker; swelling, deformity
rotting main stem Rotting trunk
I implemented the fix #32. If you look at the resulting file diffs, new auto fix and (unfortunately) field fix errors are identified:

> require_field_fix_error_file$error_name %>% table
   canker_but_no_level       rot_but_no_level status_A_but_unhealthy 
                    18                    156                   9075 
> will_auto_fix_error_file$error_name %>% table()
unhealthy_but_wrong_status 
                      9075 

Questions:

  • @mageeluke Would you be able to fix the auto fix errors?
  • @djj4tree At this point I'm guessing there is nothing we can do about the field fix errors. For sake of the error map below, how about I disable the above three field fix error checks for this year?

Sorry about this folks, this is on me :(
map_of_error_and_warnings

I can't believe there were that many A but the tree had health issues and weren't coded Alive and Unhealthy. I thought we had checks in the Fast field form to catch this. If they truly made it through they are not errors but a warning.

(Sorry for the delay. MWF are my teaching days which are more exhausting than anticipated, so I'll be devoting time to this on Tue/Thu)

I started a new branch in #33 that has both 1) the fixes to the mismatched codes in the table below and 2) Valentine's fix for DWR from yesterday.

SCBI HF
DWR Dead with resprout
canker,swelling,deformity Canker; swelling, deformity
rotting main stem Rotting trunk

Here is the updated error counts and error map currently reflected in the branch in #33. Far fewer than before

# Field fix errors
status_A_but_unhealthy    canker_but_no_level       rot_but_no_level 
                     3                     18                    156 

# Auto fix errors
unhealthy_but_wrong_status 
                         3 

map_of_error_and_warnings

@mageeluke Is my understanding correct that you will do the following?

  1. attempt to address the field fix errors from photos?
  2. auto fix errors as usual?

Thank you @rudeboybert. This makes me feel a lot better. These checks will take me a little while, but I will let you know when I've worked through them, hopefully by early next week.

Hey @mageeluke I'm making a renewed push on the paper. Is this issue still outstanding?

Hi @rudeboybert, yes, I have not checked the photos for HF yet. Just finished the semester here, I'll get to these in the next few days. Sorry!

@rudeboybert I only see the 3 stem records in "require_field_fix_error_file." Should I be looking elsewhere for the other errors?

sounds like no (sorry I forgot that the above map was updated). Sounds like we can close this issue. Here's the updated map for posterity. Thanks for checking!

map_of_error_and_warnings