nodeHeights produces non identical but identical node heights

[jacobmaugoust]

Hi Liam,

First, many thanks for this package and maintaining it. It's an euphemism to say that it's useful!

After an afternoon debugging a large function that relies on numerous other functions, I found that there is a little bug inherent to R and to computers in general (if I understand well that kind of thing) in the nodeHeights function.

When calculating the node height, the function is (line 16 of the function using trace) res[edge[i, 2]] <- res[edge[i, 1]] + el[i].

I have a case where, for a given i, the two added values are 29.3 and 22.5 and, for another i, the two added values are 41.2 and 10.6. Both result in 51.8, but both are not equal according to R (and that's due to that "floating point issue").

I suggest you replacing res[edge[i, 1]] + el[i] by as.numeric(as.character(res[edge[i, 1]] + el[i])) for instance. What I propose is clearly ugly, but that's the most straightforward thing I thought about, and everything went fine for me after that correction.

A little question, do you frequently correct some things on your github repository or do you only add several corrections and features from time to time? In other words, do you recommend rather using your CRAN package or the github one?

Thanks a lot,

Jacob

[liamrevell]

Hi Jacob.

Thanks for your kind words about phytools & this observation.

If I understand your comment, it sounds like this is a floating point value comparison issue rather than a problem with nodeHeights itself.

In general, I would not recommend comparing floating point values for equality -- regardless of whether they derive from phytools or other R packages.

Instead, you might consider asking whether two values have an absolute difference smaller than some certain small quantity -- for instance, rather than x == y you can ask abs(x-y)<=1e-12 (obviously depending on the scale of x and y).

I'm impressed by your creative hack, but I'd worry that we can't be sure how it will behave under all circumstances. You'd probably be better off simply using round to a large number of digits! (And not compare floating point values for equality to begin with, haha.)

Yes, I frequently update phytools on GitHub. For instance, between the last CRAN version & the current one, I know I pushed over 100 updates to GitHub.

All the best, Liam