thej022214 / OUwie

Estimates and compares rate differences of continuous character evolution under Brownian motion and a new set of Ornstein-Uhlenbeck (OU) models that allow the strength of selection and drift to vary across selective regimes.
8 stars 3 forks source link

OUwie currently incorrectly obtains a gibberish root age if passed a non-ultrametric tree without a root age from user #3

Closed dwbapst closed 4 years ago

dwbapst commented 6 years ago

Hello,

A recent issue raised on R-Sig-Phylo has revealed that if a root age is not given for the user-supplied tree, then a root age is instead obtained by calling max(branching.times(tree)) Apparently in some circumstances, this results in a negative estimate of the root age, and a confusing warning from the paleotree function, which is right to be utterly confused.

https://github.com/thej022214/OUwie/blob/dbce6a65b630734fcd74201b38e8a50fd7503186/R/OUwie.R#L682-L695

Unfortunately, branching.times returns gibberish when applied to non-ultrametric trees. As noted in Emmanuel's documentation for branching.times:

This function computes the branching times of a phylogenetic tree, that is the distance from each node to the tips, under the assumption that the tree is ultrametric. Note that the function does not check that the tree is effectively ultrametric, so if it is not, the returned result may not be meaningful.

Literally, it arbitrarily picks a tip, and measures the distance from the root to the other tips relative to that distance. Unless it luckily picks the tip furthest (in time) from the root on a non-ultrametric tree, its just gibberish, and often has negative values.

This same issue came up in geiger, where branching times was used to get tree depths for the Early Burst model, which meant the model was being incorrectly parameterized for non-ultrametric trees:

https://github.com/mwpennell/geiger-v2/issues/20

The correct solution here, as with that issue, is to simply replace branching.times with a call to node.depth.edgelength, which will return a vector where the max will always be the distance from the root to the furthest tip.

Cheers, -Dave Bapst

dwbapst commented 5 years ago

Is this issue really still open?

thej022214 commented 4 years ago

Yes. And I am closing it.

dwbapst commented 4 years ago

So you fixed it?

thej022214 commented 4 years ago

Yes, hence the closing.


Jeremy M. Beaulieu Assistant Professor Biological Sciences University of Arkansas (479) 575-2618 www.jeremybeaulieu.orghttp://www.jeremybeaulieu.org eeob.uark.eduhttps://eeob.uark.edu

On Apr 21, 2020, at 2:56 PM, David Bapst notifications@github.com<mailto:notifications@github.com> wrote:

So you fixed it?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_thej022214_OUwie_issues_3-23issuecomment-2D617380858&d=DwMCaQ&c=7ypwAowFJ8v-mw8AB-SdSueVQgSDL4HiiSaLK01W8HA&r=RgX4bSaNQeiQbZwLScLrzw7Ms39mF-EaNC7XeMX30u8&m=NE3IDe5qbLFaIM228bNbStXKd9Hr2fKhcW-bX5o76mE&s=BetTdmyLyStfXlMKiHPp6npzBSk7yTo0ibybWa_N068&e=, or unsubscribehttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_ACOF4FV5GZAZHVRU3KS3W5TRNX27HANCNFSM4E574HKQ&d=DwMCaQ&c=7ypwAowFJ8v-mw8AB-SdSueVQgSDL4HiiSaLK01W8HA&r=RgX4bSaNQeiQbZwLScLrzw7Ms39mF-EaNC7XeMX30u8&m=NE3IDe5qbLFaIM228bNbStXKd9Hr2fKhcW-bX5o76mE&s=UtTLzDcc5dC7NVMm-IKQw4SV5MdQV4jYfKdr7bOz7dA&e=.