AbstractHomogeneousTreeLikelihood constructor assumes root node is the last one in the nodes_ list

[JB]

Hello,

I want to draw your attention to the fact that when AbstractHomogeneousTreeLikelihood() is called on an underlying rooted tree with the option checkRooted set at true, it calls TreeTemplate::unroot(), which is fine, and then does the bookkeeping assuming that the root node is the last one added in the nodes_ internal list (AbstractHomogeneousTreeLikelihood::init_, on line 158 in AbstractHomogeneousTreeLikelihood.cpp.

One of the nice features of Bio++/Phyl is to allow the user manipulate trees, so we can add our own nodes, fiddle with the tree, etc. At the end of the day the last item in the nodes_ list may well not be the root, i.e. we can have a tree with greater node ids that the root one. As this value (true) for that option (checkRooted) is the default one, this issue can go unnoticed from many users, so it might require a fix... Or anybody adding nodes in the tree always finally reset the node indices?

Cheers,

   JB

P.S. Why is checkRooted = true the default option for TreeLikelihood constructors ? Anyway, unrooting is just having a root node with 3 sons instead of two, so reason behind this choice of always unrooting is unclear to me...

[Julien Yann Dutheil]

Hi JB

> One of the nice features of Bio++/Phyl is to allow the user manipulate
> trees, so we can add our own nodes, fiddle with the tree, etc. At the end of
> the day the last item in the nodes_ list may well not be the root, i.e. we
> can have a tree with greater node ids that the root one.

I agree that the root check is a bit brutal, however it is not as bad
as you describe ;)
The nodes_ vector is computed from a call to the Tree::getNodes
function, which operates a recursion on the tree. In this recursion,
leaves are added first, and the root last, whatever the modification
of the tree you made before building the likelihood object.

As this value
> (true) for that option (checkRooted) is the default one, this issue can go
> unnoticed from many users, so it might require a fix... Or anybody adding
> nodes in the tree always finally reset the node indices?

The recursion is fully independent of the node ids. It only depends on
the topology structure, which is specified thru the father and sons
pointers. Id are used for mapping nodes with other data (such as
likelihood arrays, scores, etc), but are not used for the recursion.

>
> Cheers,
> JB
>
> P.S. Why is checkRooted = true the default option for TreeLikelihood
> constructors ? Anyway, unrooting is just having a root node with 3 sons

It is only the case for homogeneous models, as optimizing parameters
with a rooted tree in the homogeneous case would lead to one branch
length being not identifiable.

> instead of two, so reason behind this choice of always unrooting is unclear
> to me...

I hope I could clarify this point!

Cheers,

J.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Bio++ Usage Help Forum" group.
> To post to this group, send email to biopp-he...@googlegroups.com.
> To unsubscribe from this group, send email to
> biopp-help-for...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/biopp-help-forum/-/koznT4GwSQ0J.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>



--
Julien Y. Dutheil, Ph-D
0 (+49) 6421 178 986

§ Max Planck Institute for Terrestrial Microbiology
Department of Organismic Interactions
Marburg -- GERMANY

§ Intitute of Evolutionary Sciences - Montpellier
University of Montpellier 2 -- FRANCE

[Jean-Baka Domelevo Entfellner]

Ok Julien,

Thanks for all these in-depth answers!

Best,

   JB