Negative branch length check in tree likelihood

[jordandouglas]

The tree likelihood classes should return negative infinity log-probability if any of the branches have a negative length. Running negative branch lengths through the substitution model can give unexpected results - including exceptions in the best case scenario, but sometimes +infinity likeihoods, or erroneous results in the case of compelx substitution models.

Currently, the requirement for branch lengths to be non-negative is enforced by the operators, but this is part of the model and therefore should be enforced by the calculation nodes.

[rbouckaert]

I wonder if it is possible for an operator to be reasonable, correct and produce negative branch lengths?

If not, it would be good to consistently throw an exception when this happens to make it easier for the operator to be fixed: just returning -Infinity could leave an operator bug lingering without it becoming to be known till much later.

[jordandouglas]

For example, I have had problems with origin branches, where the origin is a realparameter, and its lower bound changes with the tree. A perfectly legal tree operator could move the tree above the origin and then the origin branch has a negative length. If the origin goes through the tree likelihood as a branch, it will give problems.

Agreed that printing a warning would be helpful for developers. Currently, if an operator bug gave negative branches, the exception will typically be caught (depending on the subst model) and the developer remains unaware.