compevol / sampled-ancestors Goto Github PK

Under special circumstances, there is no node available for the SAWilsonBalding move, and the proposal methods can get stuck in a loop. A possible solution is to give up (and return Double.NEGATIVE_INFINITY) after say 100 attempts to find a candidate node.

When giving multiple trees, ConvertZBTreeToSATree created all trees for each tree reading from the file.

Maybe I created this bug earlier, but now it is fixed.

This may be a minor issue, but by convention, trees are assumed to have root node as highest numbered node, but the SAWilsonBalding operator can propose trees with root nodes of lower number since it calls Tree.setRootOnly() instead of Tree.setRoot() and the former does not automatically reorder nodes. The comment in the Tree says exactly that, but it gives no explanation why, so I am not sure why Tree.setRoot() is not called. Please let me know if you know why.

If an SAWilsonBalding proposal is followed by a NodeReight proposal in a StarBeast2 analysis, the latter does call Tree.setRoot() and changes node order so the root is highest numbered.

Perhaps other operators (SAExchange?) behave the same.

I bumped into this issue trying to find out why some caching issue on metadata of the species tree failed and caused a bit of trouble to identify the root, which I expected to be the highest numbered node.

Historically it seems that this was the done thing, but it can easily lead to problems due to the way that all classes loaded by BEAST have to share the same namespace. If someone else adds a class to their BEAST add-on that happens to have the same name (and be in the same package) as one of yours then BEAST will refuse to load either one of the add-ons. The simple fix is to move all of your packages into a root package which you have good reason to believe is unique (at least within the BEAST world) - something like sampledancestor should do it.

Wilson Balding can be optimized as suggested in issue #4

Random number seed: 1489841056910

Alignment(166strains)
166 taxa
1179 sites
93 patterns

JN635404.MVi/Virginia.USA/15.09_2009-04-06_2009.2630 = 2009.263 (6.079500000000053)
JN635407.MVi/Texas.USA/4.07_2007-01-22_2007.0603 = 2007.0603 (8.282199999999875)
KJ410048.MVi/Muenchen.DEU/19.13_2013-05-10_2013.3534 = 2013.3534 (1.9891000000000076)
...
MVs/VitSntAntao.PE.BRA/22.13_2013-06-01_2013.4137 = 2013.4137 (1.9287999999999101)

Error 110 parsing the xmljava.lang.RuntimeException: The time interval parameter should be numChanges + 1 long (10).
at beast.evolution.speciation.BirthDeathSkylineModel.getChangeTimes(Unknown Source)
at beast.evolution.speciation.BirthDeathSkylineModel.collectTimes(Unknown Source)
at beast.evolution.speciation.BirthDeathSkylineModel.initAndValidate(Unknown Source)
at beast.util.XMLParser.initBEASTObjects(Unknown Source)
at beast.util.XMLParser.parse(Unknown Source)
at beast.util.XMLParser.parseFile(Unknown Source)
at beast.app.BeastMCMC.parseArgs(Unknown Source)
at beast.app.beastapp.BeastMain.main(Unknown Source)
at beast.app.beastapp.BeastLauncher.main(Unknown Source)
input file

validate and intialize error: The time interval parameter should be numChanges + 1 long (10).

Error detected about here:

This time it can't be "the space in the file name".
Please help. Thanks you.

If one wants to run an analysis conditioning on the root when the origin input is not necessary.

Error messages look like this:

ERROR: ScaleOperator is not a valid operator for a sampled ancestor analysis.
Either remove the operator (id=TreeScaler.t:loc13162) or fix the removal probability to 1.0 so this is not a sampled ancestor analysis any more. The current analysis is not valid.
ERROR: ScaleOperator is not a valid operator for a sampled ancestor analysis.
Either remove the operator (id=TreeRootScaler.t:loc13162) or fix the removal probability to 1.0 so this is not a sampled ancestor analysis any more. The current analysis is not valid.

In a StarBEAST analysis, sampled ancestors should be in the species tree, but not in the gene trees. These operators are on the gene trees, so I believe they do not represent an error.

Proposed fix:

The SABirthDeathModel initAndValidate method has a check that generates these error messages. It should only generate them for operators that match both the type and the tree input provided to SABirthDeathModel instance.

New code should look like this:

    if (removalProbability.get() != null && removalProbability.get().getValue() < 1.0 || isSAAnalysis) {
    	// this is a sampled ancestor analysis
    	// check that there are no invalid operators in this analysis
    	List<Operator> operators = getOperators(this);
    	if (operators != null) {
    		for (Operator op : operators) {
    			boolean isOK = true;
    			if (op.getClass().isAssignableFrom(TipDatesRandomWalker.class) || 
                        (op.getClass().isAssignableFrom(SubtreeSlide.class) && ((SubtreeSlide)op).treeInput.get() == treeInput.get()) ||
                        (op.getClass().isAssignableFrom(WilsonBalding.class) && ((WilsonBalding)op).treeInput.get() == treeInput.get()) ||
                        (op.getClass().isAssignableFrom(Uniform.class) && ((Uniform)op).treeInput.get() == treeInput.get()) ||
                        (op.getClass().isAssignableFrom(Exchange.class) && ((Exchange)op).treeInput.get() == treeInput.get())) {

                            isOK = false;

    			} else if (op.getClass().isAssignableFrom(ScaleOperator.class)) {
    				// scale operators on Trees should be replaced with SAScaleOperator
    				for (StateNode o : op.listStateNodes()) {
    					if (o == treeInput.get()) {
    						isOK = false;
    					}
    				}
    			}        		 	
    			if (!isOK) {
    				Log.err.println("ERROR: " + op.getClass().getSimpleName() + 
    						" is not a valid operator for a sampled ancestor analysis.\n" + 
    						"Either remove the operator (id=" + op.getID() + ") or fix the " +
    					    "removal probability to 1.0 so this is not a sampled ancestor " +
    					    "analysis any more. The current analysis is not valid.");
    			}
    		}
    	}
    }

If one analyse a dataset of extinct species when the most recent species date is fixed and can not be sampled. A possible fix is to add the present time as an element of the tree and also condition the tree likelihood on the extinction of the clade until present.

It would be nice if an incorrect initial origin value could be automatically corrected (when possible, and with a warning) instead of an exception being thrown. Sometimes when a tree initialiser is being used it is not possible ahead of time to know what value of origin is large enough.

I suggest replacing the current exception with these two lines:

  originInput.get().setValue(rootHeight + 1.0);  
  System.err.println("WARNING: Initial value of origin was changed to " + origin() + " so that it is greater than initial root height (" + rootHeight + ")");

@rbouckaert is working on a solution that would allow a check on the RealParameter to make sure it is being estimated. Then if it is, the above solution can be employed, else throw exception.

Currently, the procedure is to specify an MRCA prior and select tipsonly=true, save the XML, and replace TipDatesRandomWalker with SampledNodeDateRandomWalker in a text editor.

Having an SampledAncestorMRCAPrior that derives from MRCAPrior and an input editor associated with it, allows this process to be all done in BEAUti, without having to deal with a text editor.

in the manual it's stated that the knowledge of the time of sample is required in order to create a tree with nodes being internal (ancestor) nodes,

is that, at all possible to avoid that requirement and to infer father - son or ancestor relation between samples without any knowledge of their time of sample?

thanks!

If I want to sample 1 - turnover but not turnover, the easy way to do is creating a new parameter oneMinusTurnover and using RPNcalculator to calculate turnover inside SABirthDeathModel. Therefore, the input of turnover has to be changed to Function.

OSX, v.2.7.4