willpearse / phest

Phenology Estimation R package
Other
5 stars 4 forks source link

Unexpected error with repeated measures #1

Open stemkov opened 5 years ago

stemkov commented 5 years ago

I'm getting an unexpected fatal error in weib.limit() when there are many repeated first measures.

For example, when I run weib.limit(rep(157,50)), I get the usual warning message about applying a correction when there are repeated earliest measurements, but also a new error reading:

Error in if (theta < ci[1] | theta > ci[2]) { : missing value where TRUE/FALSE needed

This does not happen for every case with when there are repeated earliest measurements. For example, weib.limit(rep(c(157,160,180),each=3)) does not experience the fatal error.

This appears to be happening in the "# Check for tail vs. centre of distribution issues" section which was added recently.

willpearse commented 5 years ago

Ah, I think I do know why this is happening: now that we have the new CI checking code, there is no code to check that a CI can be calculated before checking the CI (does that make sense?). Let me have a quick poke around, though, because I'm loathe to push something immediately and then make a new problem :p

willpearse commented 5 years ago

I think fixed with https://github.com/willpearse/phest/commit/d0cc19745a3b479f2072363e13bf1f9a85942dc3.

The problem was your example cannot have an estimate calculated for it (there's only one value in the entire thing), and so the missing value problem refers to how you can't run a conditional on an NA. I've added a check before that conditional to make sure that everything isn't NA before running that check; I don't think there's a need to flag anything further because, frankly, the estimates are NA in such cases and not returning a value is sufficiently clear that we couldn't calculate an estimate.

Does that make sense? If you want to write a check to make sure there is more than one observation in the input data before running the method I'm happy to merge it, but the danger with writing so many checks is that it lulls the user into the false sense that they don't need to check their input data for sense...