DiameterCorrections improvements

[ValentineHerr]

Opening this issue for discussion and for a to-do list for both myself and @VincyaneBadouard.

• [x] take out the Taper correction and make it its own function. Currently the function returns NA in DBHCor if there is only one census, which I understand if the user wants to apply the corrections that are looking for anomalies in the growth of the stems, but which is a bummer if the user only wants to apply taper corrections. I think taper correction should be its own independent function. I understand that the other diameter corrections should probably be applied once the taper correction is applied, but I think that most people applying corrections would know that too, so they would select both TaperCorrecitons and DiameterCorrections. What do you guys think (@VincyaneBadouard, @cpiponiot and @gabrielareto) ?

• [ ] I would like to have a mean of not offering the option to apply diameter (and taper) corrections if some of the necessary columns are not in the data (e.g no HOM, or no POM etc...). I need to work on that

• [x] @VincyaneBadouard, since the RequiredFormat now automatically fills in the IdStem (if not given), I think it safe to use that automatically, it will be equivalent to IdTree if the data is at the Tree level (only difference is that all trees have only one stem). So we could remove the ByStem argument, (or at least makes its default "TRUE").

• [x] @VincyaneBadouard the function needs to check that Ids actually represent 1 stem each census. I am saying that because I had a case where the function (and application) would break because I forgot to do ByStem = TRUE. It would have been nice to, instead, get an error notification saying something like "you have multiple observation for the same ID in the same Census".

[gabrielareto]

Taper correction should have its own function or routine, and work with datasets with just one census. Other corrections or flags (for extreme growth etc.) should happen, probably, after taper correction. A great change in POM should not be seen as a great change in DBH.

[ValentineHerr]

• [ ] remove MaxDBH. @VincyaneBadouard, I don't see much interest in providing a MaxDBH in the DiameterCorrections. If you feel strongly about it let me know ASAP.

• [ ] I think I am going to bring the taper correction back into this function and have add an argument for the user to decide to apply it or not, making sure that multiple censuses are not required and that it can be the only thing turned on in this function

[VincyaneBadouard]

Why do you want to delete MaxDBH? Data entry errors are the most common, and you don't want an impossibly high tree.

I didn't understand what you were talking about when you said multiple censuses for the taper correction. The taper correction just didn't need to be processed tree by tree as it just transforms from initial diameter. It can be put in the DiameterCorrection() function before calling DiameterCorrectionByTree() but you wanted to put it aside Camille (and you it seems to me) I think you should check with Camille

[ValentineHerr]

Why do you want to delete MaxDBH? Data entry errors are the most common, and you don't want an impossibly high tree.

I find that there are often one or two giant trees in a plot, trees that have much larger diameters than the rest of the population. So, I feel that removing diameter measurements > MaxDBH will either remove a true large measurement or not do anything to remove a false large measurement of a small tree. I think that your Diameter corrections should be enough to detect issues (using growth threshold etc..)

[ValentineHerr]

I am still working on getting familiar with the DiameterCorrection function.

Within this loop which I understand is supposed to be dealing with POM change, this line of code also detects abnormal growth. To me, detecting abnormal growth should be done separately from detecting POM change but there may be an important reason to do it here. @VincyaneBadouard do you remember why that part is within that loop?

Here is an outline of actions I am thinking would be appropriate, but @VincyaneBadouard and @cpiponiot let me know if that is too simplified:

1. adjust Diameter when there is a change in height of measurement
   • If there is HOM --> taper correction (and ignore POM)
   • If there is no HOM but there is POM --> IndividualDiameterShiftCorrection OR PhylogeneticHierarchicalCorrection OR RegressionInterpolation
   • If no HOM and no POM --> can't do anything
2. correct diameter where growth is abdormal (>pos threshold or < neg threshold)
   • correct with IndividualDiameterShiftCorrection OR PhylogeneticHierarchicalCorrection OR RegressionInterpolation

[VincyaneBadouard]

yes it's important to put NA at the abnormal growth at this step too because the correction prorposed at pom change is based on the other diameter values of the individual in the case of individual correction, and the last right value in the case of phylogenic correction.

"If there is HOM --> taper correction (and ignore POM)" : for me it's a user choice

RegressionInterpolation is only for punctual error If no HOM and no POM --> IndividualDiameterShiftCorrection OR PhylogeneticHierarchicalCorrection if shift error RegressionInterpolation if punctual error (see my end-of-contract document)

Why do you want to change the design we proposed? @cpiponiot

[ValentineHerr]

yes it's important to put NA

oh right, that make sense now

HOM is more precise than POM. Do you see a disadvantage of using POM only if HOM is missing?

Sorry, I don't mean to change the design, ignore my outline. I am trying to avoid having the function loop through every stem to improve speed, so I need to dig in and understand the code to be able to rewrite that part. Not having been involved with the design, I need to understand the choices that were made. Your end-of-contract document is very helpful, but it does not explain some of the decisions like: Why not also propose RegressionInterpolation for POM change?