CompEvol / beast2

Bayesian Evolutionary Analysis by Sampling Trees
www.beast2.org
GNU Lesser General Public License v2.1
237 stars 83 forks source link

Incorrect XML for up down operator of clock rate and tree for multi-alignment #228

Open walterxie opened 10 years ago

walterxie commented 10 years ago

BEAUti generates incorrect up down operator of clock rate and tree for multi-alignment

Load multi-alignment, link clock rate but unlink trees, make clock rate be estimated, and then the xml should have up-down for each clock and tree, and a big up-down for all clocks and trees together:

For example, 3 genes:

<operator id="strictClockUpDownOperator.c:Pad_R000255_Scaffold10" scaleFactor="0.9" optimise="false" spec="UpDownOperator" weight="15.0">
    <parameter idref="clockRate.c:Pad_R000255_Scaffold10" name="up"/>
    <tree idref="Tree.t:Pad_R000255_Scaffold10" name="down"/>
</operator>

<operator id="strictClockUpDownOperator.c:Pad_R000272_Scaffold10" scaleFactor="0.9" optimise="false" spec="UpDownOperator" weight="15.0">
    <parameter idref="clockRate.c:Pad_R000255_Scaffold10" name="up"/>
    <tree idref="Tree.t:Pad_R000272_Scaffold10" name="down"/>
</operator>

<operator id="strictClockUpDownOperator.c:Pad_R000333_Scaffold10" scaleFactor="0.9" optimise="false" spec="UpDownOperator" weight="15.0">
    <parameter idref="clockRate.c:Pad_R000255_Scaffold10" name="up"/>
    <tree idref="Tree.t:Pad_R000333_Scaffold10" name="down"/>
</operator>

<operator id="strictClockUpDownOperator.all" scaleFactor="0.95" optimise="false" spec="UpDownOperator" weight="15.0">
    <parameter idref="clockRate.c:Pad_R000255_Scaffold10" name="up"/>
    <tree idref="Tree.t:Pad_R000255_Scaffold10" name="down"/>
    <tree idref="Tree.t:Pad_R000272_Scaffold10" name="down"/>
    <tree idref="Tree.t:Pad_R000333_Scaffold10" name="down"/>
</operator>
rbouckaert commented 7 years ago

The above commits should have fixed the issue, and tests are running fine on my Linux machine, but Hudson does not agree. Still investigating what is going wrong.

walterxie commented 6 years ago

@rbouckaert https://github.com/CompEvol/beast2/commit/733b45177481df913a5554251bd418e31e188857 creates a bug for this issue. When linking clock rate but unlinking tree, the template only creates 1 up-down, because n=1.

<!-- need updown operator for clockRate?!? Also in SubstModel.xml -->
<upDownOperator id='strictClockUpDownOperator.c:$(n)' spec='UpDownOperator' scaleFactor="0.75" weight="3">
   <up idref="clockRate.c:$(n)"/>
   <down idref="Tree.t:$(n)"/>
</upDownOperator>

Is it possible in template to allow the different number of clock rates and trees?

walterxie commented 6 years ago

related to #211

rbouckaert commented 6 years ago

Sharing a clock but not the trees almost certainly implies use of a strict clock model (what other model could be reasonable?). Therefore, a single up-down operator that scales all trees as well as the clock would make sense. Not sure how having multiple up-down operators helps in this situation.

walterxie commented 6 years ago

But the current template is only worked for a single tree, because of $(n) after Tree.t:. When we are talking about all trees, they should be m downs from Tree.t:1 to the last tree Tree.t:m, where n = [1,m].

alexeid commented 6 years ago

Conceptually it would make sense to share the global parameters of a relaxed clock, but obviously not the individual rates on branches which would have to be different for the different trees.

On 14/03/2018, at 3:29 PM, Remco Bouckaert notifications@github.com wrote:

Sharing a clock but not the trees almost certainly implies use of a strict clock model (what other model could be reasonable?). Therefore, a single up-down operator that scales all trees as well as the clock would make sense. Not sure how having multiple up-down operators helps in this situation.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CompEvol/beast2/issues/228#issuecomment-372884163, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3WSfRbCQ4gxU8aSPlm6cmum9HPYwE9ks5teICHgaJpZM4CnGoe.