bioDS / beast-phylonco

A BEAST2 package for single-cell cancer evolution
GNU Lesser General Public License v2.1
6 stars 2 forks source link

Duplicate relaxed clock operator created by lphybeast #5

Open kche309 opened 2 years ago

kche309 commented 2 years ago

Running LPhyBeast on an lphy script with a relaxed clock model and error model will generate duplicate clock operators.

lphy script: simpleRelaxedError_lphy.txt

lphybeast output: simpleRelaxedError_xml.txt

@walterxie has worked on a solution by removing the duplicated original Tree and Alignment objects.

The object duplication is because constructTreeAndBranchRate() is called twice, once when constructing PhyloCTMC for the generic non-error tree, and another time by GT16ErrorModelToBeast which constructs a tree with an error model. Internally this duplicates the tree and branch objects and operators.

The ideal solution would be to modify the code to construct a tree only once, or by passing the error model into the tree construction method.

The temporary solution is to add a flag to constructTreeAndBranchRate() in PhyloCTMCToBEAST.java to optionally turn off branch rate operators during tree construction.

https://github.com/kche309/LPhyBeast/blob/6bfa606bc132bc3ecaf42598f15a23be0f34d881/src/lphybeast/tobeast/generators/PhyloCTMCToBEAST.java#L158

kche309 commented 2 years ago

Comments from @alexeid The logic that duplicates should be fixed at the fundamental level. The fundamental logic should be a single traversal of the graphical model that creates one BEASTObject for every Node.

walterxie commented 1 year ago

Your data block is illegal, change to this:

data {
  L = 200;
  taxa = taxa(names=1:16);
  numBranches = 30;
}
walterxie commented 1 year ago

The only solution to fix this "at the fundamental level", that I can think of, is to make GT16ErrorModel inherit PhyloCTMC and add two parameters epsilon and delta, and then overwrite public RandomVariable<Alignment> sample() {.

Any better ideas?