macroevolution / bamm

A program for multimodel inference on speciation and trait evolution
GNU General Public License v2.0
33 stars 10 forks source link

Checks prior to running BAMM #43

Closed jeffjshi closed 10 years ago

jeffjshi commented 10 years ago

In the course of running my datasets it seems to me there should be slightly more informative error messages on some basic tree issues so that people can fix them. The three things that have come up most often are: non-bifurcating trees, negative branch lengths (or 0 branch lengths), and malformed trees with internal nodes. Right now they give pretty uninformative error messages before aborting (or segmentation faulting), and it seems maybe these should be checked in the tree structure before actually attempting to run BAMM. Is it possible to do that within C? I know it's fairly easy to check in R first.

redcurry commented 10 years ago

Could you tell us what error messages you're currently getting?

jeffjshi commented 10 years ago

I think the only two I've seen are "error in Node::computeSpeciationRateIntervalRelTime - times are bad..." for negative branch lengths and a flat segmentation fault on malformed trees with internal nodes/non-bifurcating trees. For the former, however, they occur after BAMM runs for a variable number of generations (anywhere from 1 to 50,001 as far as I've seen).

redcurry commented 10 years ago

Ok, I'll need to see the trees that are malformed to help me identify where to put an appropriate error message.

jeffjshi commented 10 years ago

I could send you a few or all of them if you'd like. Let me know - I can zip them up.

redcurry commented 10 years ago

All of them would be best, but it's not a GB right? =)

jeffjshi commented 10 years ago

Nah, these are small. I'll zip them up and email them to you shortly, with BAMM and screen output included.

redcurry commented 10 years ago

I just implemented the check for negative branch lengths. Check with a few trees if you can. Thanks!

jeffjshi commented 10 years ago

Just tried the new version with just the folders with negative branch lengths. No good, it seems...same error on all of them.

jeffjshi commented 10 years ago

Actually to be more accurate, some of them seem to run even though they do have negative branch lengths. Most simply abort, though.

redcurry commented 10 years ago

By the way, I made the change on the master branch, not the mcmc branch, just in case you're running things from the mcmc branch.

On Wed, Jan 8, 2014 at 1:59 PM, jeffjshi notifications@github.com wrote:

Actually to be more accurate, some of them seem to run even though they do have negative branch lengths. Most simply abort, though.

— Reply to this email directly or view it on GitHubhttps://github.com/macroevolution/bamm/issues/43#issuecomment-31865925 .

jeffjshi commented 10 years ago

Yeah I double-checked that. Still no good...

jeffjshi commented 10 years ago

OK to recap. Ultrametric and <0 branch length checks currently good. Last issue in this dataset is the internal node tree, which still segmentation faults but will write .txt files as output. For reference, tree 261

redcurry commented 10 years ago

Ok, please check again tree 261... The problem was that the tree contained a node with a single child node, so it wasn't strictly bifurcating. I implemented a check for this and should stop the program with an error rather than crash.

On Wed, Jan 8, 2014 at 5:56 PM, jeffjshi notifications@github.com wrote:

OK to recap. Ultrametric and <0 branch length checks currently good. Last issue in this dataset is the internal node tree, which still segmentation faults but will write .txt files as output. For reference, tree 261

— Reply to this email directly or view it on GitHubhttps://github.com/macroevolution/bamm/issues/43#issuecomment-31885422 .

jeffjshi commented 10 years ago

Yep, that works. I can't think of any other pre-analysis checks that should be run on the trees at this point, so this issue looks solid for now.