missing model for a branch ?

[Laurent Guéguen]

Hi,

I have a branch assignment problem with partnh, on an example on which I had no problem in July.
Here is the end of the output:

* Homogeneous model - LogL.............: -4861.52
                    - df...............: 20
Parameter found........................: 1_Full.theta=0.443679
Parameter found........................: 1_Full.theta1=0.608931
Parameter found........................: 1_Full.theta2=0.618199
Parameter found........................: 2_Full.theta=0.408404
Parameter found........................: 2_Full.theta1=0.700861
Parameter found........................: 2_Full.theta2=0.402973
Parameter found........................: 3_Full.theta=0.3962
Parameter found........................: 3_Full.theta1=0.582331
Parameter found........................: 3_Full.theta2=0.594998
Root frequencies ......................: F3X4
Stationarity assumed...................: no
Global parameter.......................: CodonDistPhasFreq.beta
Global parameter.......................: CodonDistPhasFreq.2_Full.theta
Global parameter.......................: CodonDistPhasFreq.2_Full.theta2
Global parameter.......................: CodonDistPhasFreq.1_Full.theta
Global parameter.......................: CodonDistPhasFreq.1_Full.theta2
Global parameter.......................: CodonDistPhasFreq.2_Full.theta1
Global parameter.......................: CodonDistPhasFreq.1_Full.theta1
Global parameter.......................: CodonDistPhasFreq.3_Full.theta2
Global parameter.......................: CodonDistPhasFreq.3_Full.theta1
Global parameter.......................: CodonDistPhasFreq.123_K80.kappa
AIC....................................: 9763.04
AICc...................................: 9764.27
BIC....................................: 9854.15
Likelihood comparison method...........: BIC
Stop condition.........................: test 20 nested models after local best
Testing non-stationary model...........: yes
Number of real partitions..............: 1
Partition 1............................: 8 element(s).
SubstitutionModelSet::addParameters. This node has no associated model: 6

whereas the tree has 10 nodes.
Any hints?

Thanks,
Laurent

[Julien Yann Dutheil]

Hi Laurent,

As far as I know, no modification was performed since that time (at
least I did not touch that code).
If you send me the data files I can give it a look.

J.

> --
>
>
>



--
Julien Y. Dutheil, Ph-D
0 (+49) 6421 178 530

§ Max Planck Institute for Terrestrial Microbiology
Department of Organismic Interactions
Marburg -- GERMANY

§ Intitute of Evolutionary Sciences - Montpellier
University of Montpellier 2 -- FRANCE

[Laurent Guéguen]

Hi Julien,

actually I have EXACTLY the same problem with the LysosymeLarge
example (in the case of cluster_free):

SubstitutionModelSet::addParameters. This node has no associated
model: 6

Strange, isn't it?

Cheers, L

[Laurent Guéguen]

Hi again,

if anybody is debugging it, just a hint (with LysosymeLarge example),
node 6 is missing in lysozymeLarge.cluster_free.dnd, which is the
output of mapnh.

A+,
L

[Julien Yann Dutheil]

Hi Laurent,

I did not touch this code for a long time, neither the code for
substitution models and co, so I don't know what happened there... I
will give it a look one of those days. If you want, in between, there
is still the unit test for codon simulations that does not work
because of some changes in the codon frequency classes (cf previous
emails some time ago..) :D

Cheers,

Julien.

[Laurent Guéguen]

Hi Julien,

do not worry, I am working on it this evening,
but it was only a hint if anyone was on it too.

A+,
L

On Sep 13, 5:06 pm, Julien Yann Dutheil <jy.duth...@gmail.com> wrote:
> Hi Laurent,
>

[Laurent Guéguen]

Hi again,

I found it, I think it is very easy to fix it, but since I am not sure
I would correctly fix this:

In AbstractAgglomerativeDistanceMethod, rootTree_ is false by default
( and a method to change it is missing).
So in AbstractAgglomerativeDistanceMethod::computeTree, the size of
currentNodes_ at the end is 3.
But in MultinomialClustering::finalStep, only two sons of the root are
considered to build the tree, and so a part
of the clustering tree is missing.

A+,
L

[Julien Yann Dutheil]

Hi Laurent,

Then I have to apologize, because this is a change I did recently to
fix UPGMA which was buggy. I did not expect it to have consequences
here... I think the multinomial clustering should pass the correct
rootTree value to the abstract method. I will add the method to change
the value of the boolean outside the constructor.

Sorry again for the mess,

Julien.

[Laurent Guéguen]

Hi Julien,

no problem, actually nowadays the graph of dependencies between
classes is rather complex.
Thanks for the fix,
a+,
L

On Sep 14, 7:46 am, Julien Yann Dutheil <jy.duth...@gmail.com> wrote:
> Hi Laurent,
>

[Julien Yann Dutheil]

Hi Laurent and Tristan,

Sorry to come back late with this. If the fix proposed by Laurent works, then I think the files in the Mantelids example should be up to date. Will do the others later.

Laurent, do you need setRootedTree(bool) in the abstract agg method? It seems dangerous to me, I would like to move it to the derived classes. Using it with false on UPGMA for instance would potentially crash it... and some methods cannot offer the choice of a (un)rooted tree...

Cheers,

Julien.

[Tristan Lefebure]

Hello,
Thanks Julien for the fix. I am running into the following problem with the Mantelids examples from the latest code. The log testNH log file reports identical likelihood values for all the models:

MantellidDataset.model_BIC.log.txt

Threshold       NbPartitions    LogL    DF      AIC     AICc    BIC
0       1       -45437.4        42      90958.8 91184.8
0       2       -45437.4        42      90958.8 90961.1 91184.8
0.000546        5       -45437.4        42      90958.8 90961.1 91184.8
0.0065698       7       -45437.4        42      90958.8 90961.1 91184.8
0.12166 9       -45437.4        42      90958.8 90961.1 91184.8
0.214856        11      -45437.4        42      90958.8 90961.1 91184.8
0.22911 12      -45437.4        42      90958.8 90961.1 91184.8
0.245656        13      -45437.4        42      90958.8 90961.1 91184.8
0.444636        14      -45437.4        42      90958.8 90961.1 91184.8
0.473002        16      -45437.4        42      90958.8 90961.1 91184.8
0.54448 19      -45437.4        42      90958.8 90961.1 91184.8
0.646383        21      -45437.4        42      90958.8 90961.1 91184.8
0.68854 22      -45437.4        42      90958.8 90961.1 91184.8
0.699006        23      -45437.4        42      90958.8 90961.1 91184.8
0.802248        24      -45437.4        42      90958.8 90961.1 91184.8

(I also had to change the following to have it work:
[tristan@umr5023-proasellus MantellidFrogs] diff -u PartNH.bpp /opt/src/bpp/sources/testnh/examples/MantellidFrogs/PartNH.bpp
--- PartNH.bpp  2012-09-23 21:56:43.116754809 +0200
+++ /opt/src/bpp/sources/testnh/examples/MantellidFrogs/PartNH.bpp      2012-09-17 08:48:10.978306600 +0200
@@ -4,7 +4,7 @@
 #MapNH can output such an NHX tree if the input one was in Newick or Nexus.
 input.tree.file = $(DATA).nhx
 
-input.cluster_tree.file = MantellidDataset.cluster_join.dnd
+input.cluster_tree.file = Clusters.dnd
 
 #A simple newick tree showing the resulting partitions for visual inspection:
 output.partitions.file = Partitions.dnd
)

Am I missing something obvious or does the example file still not right?

Thanks!

--
Tristan

[Julien Yann Dutheil]

Hi Tristan,

It should be fixed now.

@Laurent: the syntax *theta* for specifying parameters is not working
yet, right?

J.

[Laurent Guéguen]

Hi folks,

yes indeed, now you can put any stars as you want in the descrption
of the parameter names, such as MixedModel.*theta*

I have done it only for stars, but I am waiting for regexp libraries
to
be at last included into the stl.

A+,
L

On Sep 25, 3:15 pm, Julien Yann Dutheil <jy.duth...@gmail.com> wrote:
> Hi Tristan,
>
> It should be fixed now.
>
> @Laurent: the syntax *theta* for specifying parameters is not working
> yet, right?
>
> J.
>
> On Mon, Sep 24, 2012 at 2:56 PM, Tristan Lefebure
>

> ...
>
> read more »

[Laurent Guéguen]

Hi J,

actually I have added this function already, but I remove it then.

Cheers,
L

[Julien Yann Dutheil]

Hi Laurent,

It does not seem to work, as if I write the shared parameters and
parameters to ignore as YN98.*theta*, partnh does not work, while if I
write YN98.1_Full.theta*, YN98.2_Full.theta*, YN98.3_Full.theta*,
everything goes well (you can see it in the Mantellids data set).

Best,

Julien.

[Tristan Lefebure]

Hello,
It's running fine now! Thanks.

Still a few things:

- on the second line of the partNH log file, the AICc (not the BIC) is missing (from the Mantellid example data set):

Threshold       NbPartitions    LogL    DF      AIC     AICc    BIC
0       1       -45437.4        42      90958.8 91184.8

0       2       -45425.8        43      90937.6 90939.9 91169
0.000546        5       -45388.1        46      90868.2 90870.6 91115.7
0.0065698       7       -45381  48      90858   90860.8 91116.3
...

- on my dataset, not the Mantellid one, I can not reproduce the AICc results:

Threshold

NbPartitions

LogL

DF

AIC

AICc

BIC

5.08e-05

4

-13424.4

207

27262.8

28842.1

28000.6

The number of site is 261

Using the following formula:

AIC <- function(lnL, K) { -2*lnL + 2*K }
AICc <- function(AIC, K, n) {  AIC + 2*K*(K + 1)/(n - K - 1)  }
BIC <- function(lnL, K, n) { -2*lnL + K*log(n) }

I am getting for this model:

lnL = -13424.4
K = 207
n = 261

> AIC(lnL, K)
[1] 27262.8
> AICc(AIC(lnL, K), K, n)
[1] 28887.55
> BIC(lnL, K, n)
[1] 28000.66

I am quite puzzled by this one (since it works fine for BIC and on the example dataset)

Cheers,
--
Tristan

[Laurent Guéguen]

Hi Julien,

actually I changed it in bpp-phyl files, but not in partnh.
And it seems that the management of * is done specifically in
partnh.cpp
in method:

SubstitutionModelSet* buildModelSetFromPartitions(

I correct it.
By the way, is there another place where there this is done?
L

On Sep 26, 7:08 am, Julien Yann Dutheil <jy.duth...@gmail.com> wrote:
> Hi Laurent,
>

[Julien Yann Dutheil]

Hi Laurent,

Sorry, I did not remember the code was duplicated there. I think it is
the only place.

J.

[Julien Yann Dutheil]

Hi Tristan,

Thanks a lot for reporting these issues. I think I have fixed the bugs (hopefully).

Best,

Julien.

--
 
 
 

[Laurent Guéguen]

HI,

no problem, it is fixed now.

By the way, I think we should avoid this kind of duplication of code
because it is a complication for updating.

Also, all the big stuff for reading the configuration file is
multiplied
in numerous programs.

Cheers,
L

On Sep 27, 8:06 am, Julien Yann Dutheil <jy.duth...@gmail.com> wrote:
> Hi Laurent,
>

> Sorry, I did not remember the code was duplicated there. I think it is
> the only place.
>
> J.
>
> On Wed, Sep 26, 2012 at 10:47 PM, Laurent Guéguen
>

> ...
>
> read more »

[Julien Yann Dutheil]

On Fri, Sep 28, 2012 at 6:55 AM, Laurent Guéguen
<lau.g...@laposte.net> wrote:
> HI,
>
> no problem, it is fixed now.
>
> By the way, I think we should avoid this kind of duplication of code
> because it is a complication for updating.

I do agree, but it was duplicated probably for some reason I cannot
remember. I must have had to modify something from the original code
that was specific to partnh...

>
> Also, all the big stuff for reading the configuration file is
> multiplied
> in numerous programs.

The application tools are meant to reduce that issue, maybe we need
more function in those if several things are common between programs.

J.

[Tristan Lefebure]

Thanks Julien,

Almost there! PartNH runs fine. I am now stuck with an upstream problem: bppml (so probably not a testNH issue, rather a bpp one)

Bppml crashes while reading the trees:

-----
Input tree.............................: user
DEVEL warning: Node::addSon. Son node already registered! No pb here, but could be a bug in your implementation...
DEVEL warning: Node::addSon. Son node already registered! No pb here, but could be a bug in your implementation...
DEVEL warning: Node::addSon. Son node already registered! No pb here, but could be a bug in your implementation...
Node::addSon(). Empty node given as input.
-----

Ok, so I though this might just be a library version issue. So I updated all the bpp-*, but got the following compilation problem with bpp-seq:

[tristan@umr5023-proasellus bpp-seq] sudo make install
[ 47%] Built target bppseq-shared
[ 95%] Built target bppseq-static
[ 96%] Built target test_alignment_scores
[ 96%] Built target test_alphabets
[ 97%] Built target test_bowker
[ 98%] Built target test_containers
[ 98%] Building CXX object test/CMakeFiles/test_io.dir/test_io.cpp.o
/opt/src/bpp/sources/bpp-seq/test/test_io.cpp: In function ‘int main()’:
/opt/src/bpp/sources/bpp-seq/test/test_io.cpp:52: error: ‘class bpp::Fasta’ has no member named ‘readAlignment’
/opt/src/bpp/sources/bpp-seq/test/test_io.cpp:60: error: ‘class bpp::Mase’ has no member named ‘readAlignment’
/opt/src/bpp/sources/bpp-seq/test/test_io.cpp:62: error: ‘class bpp::Clustal’ has no member named ‘readAlignment’
/opt/src/bpp/sources/bpp-seq/test/test_io.cpp:64: error: ‘class bpp::Phylip’ has no member named ‘readAlignment’
/opt/src/bpp/sources/bpp-seq/test/test_io.cpp:66: error: ‘class bpp::Phylip’ has no member named ‘readAlignment’
make[2]: *** [test/CMakeFiles/test_io.dir/test_io.cpp.o] Erreur 1
make[1]: *** [test/CMakeFiles/test_io.dir/all] Erreur 2
make: *** [all] Erreur 2


Whatever, this might be unrelated, I just skipped this one, and went to recompile bppsuite (v0.7.0, no git repo, right?) with the (almost) latest bpp libraries and got this third nasty one:

[tristan@umr5023-proasellus bppsuite-0.7.0] sudo make install
[  0%] Built target info
[  0%] Built target man
[  9%] Built target bppalnscore
[ 18%] Built target bppancestor
[ 27%] Built target bppconsense
[ 36%] Building CXX object bppSuite/CMakeFiles/bppdist.dir/bppDist.cpp.o
/opt/src/bpp/bppsuite-0.7.0/bppSuite/bppDist.cpp: In function ‘int main(int, char**)’:
/opt/src/bpp/bppsuite-0.7.0/bppSuite/bppDist.cpp:218: error: no matching function for call to ‘bpp::OptimizationTools::buildDistanceTree(bpp::DistanceEstimation&, bpp::AgglomerativeDistanceMethod&, bpp::ParameterList&, bool, bool, std::string&, double&, unsigned int&, bpp::OutputStream*&, bpp::OutputStream*&, unsigned int&)’
/usr/local/include/Bpp/Phyl/OptimizationTools.h:728: note: candidates are: static bpp::TreeTemplate<bpp::Node>* bpp::OptimizationTools::buildDistanceTree(bpp::DistanceEstimation&, bpp::AgglomerativeDistanceMethod&, const bpp::ParameterList&, bool, const std::string&, double, unsigned int, bpp::OutputStream*, bpp::OutputStream*, unsigned int)
/opt/src/bpp/bppsuite-0.7.0/bppSuite/bppDist.cpp:311: error: no matching function for call to ‘bpp::OptimizationTools::buildDistanceTree(bpp::DistanceEstimation&, bpp::AgglomerativeDistanceMethod&, bpp::ParameterList&, bool&, bool, std::string&, double&, unsigned int&, NULL, NULL, int)’
/usr/local/include/Bpp/Phyl/OptimizationTools.h:728: note: candidates are: static bpp::TreeTemplate<bpp::Node>* bpp::OptimizationTools::buildDistanceTree(bpp::DistanceEstimation&, bpp::AgglomerativeDistanceMethod&, const bpp::ParameterList&, bool, const std::string&, double, unsigned int, bpp::OutputStream*, bpp::OutputStream*, unsigned int)
make[2]: *** [bppSuite/CMakeFiles/bppdist.dir/bppDist.cpp.o] Erreur 1
make[1]: *** [bppSuite/CMakeFiles/bppdist.dir/all] Erreur 2
make: *** [all] Erreur 2


Oh well, I am not sure if these compilation crashes are related, but I am surely lost...

--
Tristan

[Tristan Lefebure]

Ok, forget this one. I just discovered the SVN repo for bppsuite, and it compiles fines. Then bppml runs fine. Great!

(that doesn't resolve the bpp-seq compilation crash though)

Sorry for the false positive
--
Tristan

[Julien Yann Dutheil]

Hi Tristan,

For bppml, did you compile it yourself from svn or are you using packages from the latest stable version?

For compiling the libs, it is indeed a good idea to update them from git. The error you get is due to unit tests, which build against the installed library, and not the newly compiled one. You have two solutions:

- As suggested by the warning that cmake must have given you, you should remove any installed library before recompiling.

- Or, maybe easier, first desactivate unit testing, then make install, then reactivate unit testing, then make again. You can desactivate unit testing with cmake -DBUILD_TEST=OFF, or using ccmake.

Everything should compile fine after that. This is due to recent changes in the interface of bpp-seq and bpp-phyl.

Cheers,

Julien.

[Julien Yann Dutheil]

Ok. See previous email for the compilation issue.

J.