From 2c9b6fae2468d8067765ae39689a17c0dad153b4 Mon Sep 17 00:00:00 2001 From: RevBayes analysis <1695515+ms609@users.noreply.github.com> Date: Wed, 5 Feb 2025 16:26:35 +0000 Subject: [PATCH 1/7] Functionalize existing (erroneous) behaviour --- R/AdditionTree.R | 10 +++++++--- tests/testthat/test-AdditionTree.R | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/R/AdditionTree.R b/R/AdditionTree.R index 29139ec1d..ef973774c 100644 --- a/R/AdditionTree.R +++ b/R/AdditionTree.R @@ -31,7 +31,7 @@ AdditionTree <- function (dataset, concavity = Inf, constraint, sequence) { # Initialize missing parameters taxa <- names(dataset) if (missing(sequence)) { - sequence <- taxa[1] + sequence <- taxa[[1]] } else if (is.numeric(sequence)) { sequence <- taxa[sequence] } @@ -78,7 +78,7 @@ AdditionTree <- function (dataset, concavity = Inf, constraint, sequence) { constraint <- MatrixToPhyDat(t(as.matrix(constraint))) } thisConstr <- constraint[theseTaxa] - if (length(thisConstr[[1]]) && min(table(unlist(thisConstr))) > 1) { + if (.ConstraintConstrains(thisConstr)) { # Constraint constrains theseTaxa morphyConstr <- PhyDat2Morphy(thisConstr) @@ -114,6 +114,10 @@ AdditionTree <- function (dataset, concavity = Inf, constraint, sequence) { tree } -.Recompress <- function (dataset) { +.ConstraintConstrains <- function(constraint) { + length(constraint[[1]]) && min(table(unlist(constraint))) > 1 +} + +.Recompress <- function(dataset) { MatrixToPhyDat(PhyDatToMatrix(dataset)) } diff --git a/tests/testthat/test-AdditionTree.R b/tests/testthat/test-AdditionTree.R index 3c943504a..dd7846e11 100644 --- a/tests/testthat/test-AdditionTree.R +++ b/tests/testthat/test-AdditionTree.R @@ -20,6 +20,24 @@ test_that("Addition tree is more parsimonious", { expect_lt(Score(pr, "pr"), Score(nj10, "pr")) }) +test_that(".ConstraintConstrains() succeeds", { + expect_false(.ConstraintConstrains( + structure(list(A = 1L, B = 2L, C = 1L, D = 1L, E = 3L), weight = 1L, nr = 1L, + nc = 2L, index = 1L, levels = 0:1, + allLevels = c("0", "1", "?"), type = "USER", + contrast = structure(c(1, 0, 1, 0, 1, 1), dim = 3:2, + dimnames = list(NULL, 0:1)), + class = "phyDat") + )) + expect_true(.ConstraintConstrains(structure( + list(A = 1L, B = 2L, C = 1L, D = 1L, E = 3L, F = 2L), weight = 1L, nr = 1L, + nc = 2L, index = 1L, levels = 0:1, allLevels = c("0", "1", "?"), + type = "USER", contrast = structure(c(1, 0, 1, 0, 1, 1), dim = 3:2, + dimnames = list(NULL, 0:1)), + class = "phyDat") + )) +}) + test_that("Addition tree obeys constraints", { dataset <- MatrixToPhyDat(matrix( c(0, 1, 1, 1, 0, 1, From d57f0c84c84204f50116552629972fa85ff23056 Mon Sep 17 00:00:00 2001 From: RevBayes analysis <1695515+ms609@users.noreply.github.com> Date: Wed, 5 Feb 2025 16:27:06 +0000 Subject: [PATCH 2/7] Check NULL --- tests/testthat/test-AdditionTree.R | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testthat/test-AdditionTree.R b/tests/testthat/test-AdditionTree.R index dd7846e11..16fe416df 100644 --- a/tests/testthat/test-AdditionTree.R +++ b/tests/testthat/test-AdditionTree.R @@ -21,6 +21,7 @@ test_that("Addition tree is more parsimonious", { }) test_that(".ConstraintConstrains() succeeds", { + expect_false(.ConstraintConstrains(NULL)) expect_false(.ConstraintConstrains( structure(list(A = 1L, B = 2L, C = 1L, D = 1L, E = 3L), weight = 1L, nr = 1L, nc = 2L, index = 1L, levels = 0:1, From cb07ec4584db7975d22cc7fd74df13422c0d651b Mon Sep 17 00:00:00 2001 From: RevBayes analysis <1695515+ms609@users.noreply.github.com> Date: Wed, 5 Feb 2025 16:58:56 +0000 Subject: [PATCH 3/7] .ConstraintConstrains implementation --- DESCRIPTION | 2 +- NEWS.md | 5 +++++ R/AdditionTree.R | 16 +++++++++++++++- tests/testthat/test-AdditionTree.R | 15 +++++++++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index f688aa6b6..dd8cc1a7d 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: TreeSearch Title: Phylogenetic Analysis with Discrete Character Data -Version: 1.5.1.9002 +Version: 1.5.1.9003 Authors@R: c( person( "Martin R.", 'Smith', diff --git a/NEWS.md b/NEWS.md index 471dfabfc..a9a91ffd6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,8 @@ +# TreeSearch 1.5.1.9003 (2025-02) + +- Improve support for constraints in `AdditionTree()` + (#173)[https://github.com/ms609/TreeSearch/issues/173] + # TreeSearch 1.5.1.9002 (2025-01) - Buttons to download consensus trees in app diff --git a/R/AdditionTree.R b/R/AdditionTree.R index ef973774c..05191cfc8 100644 --- a/R/AdditionTree.R +++ b/R/AdditionTree.R @@ -115,7 +115,21 @@ AdditionTree <- function (dataset, concavity = Inf, constraint, sequence) { } .ConstraintConstrains <- function(constraint) { - length(constraint[[1]]) && min(table(unlist(constraint))) > 1 + if (length(constraint[[1]]) < 1) { + FALSE + } else { + cont <- `mode<-`(attr(constraint, "contrast"), "logical") + nLevel <- dim(cont)[[1]] + # Could be > 2× more efficient using lower.tri + exclude <- vapply(seq_len(nLevel), function(i) { + colSums(apply(cont, 1, `&`, cont[i, ])) == 0 + }, logical(nLevel)) + + # TODO Not sure about this; passes tests but not proven to + # work for all odd edge cases, e.g. 02 03 1 1 + splits <- exclude * tabulate(unlist(constraint), nLevel) + any(splits[lower.tri(splits)] > 1 & t(splits)[lower.tri(splits)] > 1) + } } .Recompress <- function(dataset) { diff --git a/tests/testthat/test-AdditionTree.R b/tests/testthat/test-AdditionTree.R index 16fe416df..91b863cbe 100644 --- a/tests/testthat/test-AdditionTree.R +++ b/tests/testthat/test-AdditionTree.R @@ -37,6 +37,21 @@ test_that(".ConstraintConstrains() succeeds", { dimnames = list(NULL, 0:1)), class = "phyDat") )) + expect_false(.ConstraintConstrains(structure( + list(A = 1L, B = 2L, C = 1L, D = 1L, E = 3L, F = 2L), weight = 1L, nr = 1L, + nc = 2L, index = 1L, + levels = 0:2, allLevels = c("0", "1", "2", "?"), type = "USER", + contrast = structure(c(1, 1, 0, 1, 0, 1, 0, 1, 0, 0, 1, 1), + dim = c(4, 3), dimnames = list(NULL, 0:2)), + class = "phyDat") + )) + expect_true(.ConstraintConstrains(structure( + list(A = 1L, B = 2L, C = 1L, D = 1L, E = 3L, F = 2L), weight = 1L, nr = 1L, + nc = 2L, index = 1L, levels = 0:2, allLevels = c("0", "1", "2", "?"), + type = "USER", contrast = structure(c(1, 0, 1, 1, 0, 1, 0, 1, 1), + dim = c(3, 3), dimnames = list(NULL, 0:2)), + class = "phyDat") + )) }) test_that("Addition tree obeys constraints", { From 38a8208deec5e05d92615fd9da26177c81507807 Mon Sep 17 00:00:00 2001 From: RevBayes analysis <1695515+ms609@users.noreply.github.com> Date: Wed, 5 Feb 2025 17:00:29 +0000 Subject: [PATCH 4/7] Comment --- R/AdditionTree.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/AdditionTree.R b/R/AdditionTree.R index 05191cfc8..3290406b9 100644 --- a/R/AdditionTree.R +++ b/R/AdditionTree.R @@ -125,8 +125,8 @@ AdditionTree <- function (dataset, concavity = Inf, constraint, sequence) { colSums(apply(cont, 1, `&`, cont[i, ])) == 0 }, logical(nLevel)) - # TODO Not sure about this; passes tests but not proven to - # work for all odd edge cases, e.g. 02 03 1 1 + # TODO Validate; passes existing tests, but these do not include all + # edge cases, e.g. 02 03 1 1 splits <- exclude * tabulate(unlist(constraint), nLevel) any(splits[lower.tri(splits)] > 1 & t(splits)[lower.tri(splits)] > 1) } From 1091ee224f2b331efa515ecbac1d8e10abe0e949 Mon Sep 17 00:00:00 2001 From: RevBayes analysis <1695515+ms609@users.noreply.github.com> Date: Wed, 5 Feb 2025 17:05:54 +0000 Subject: [PATCH 5/7] 1-col constraints --- R/AdditionTree.R | 29 ++++++++++++++++++----------- tests/testthat/test-AdditionTree.R | 10 ++++++++++ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/R/AdditionTree.R b/R/AdditionTree.R index 3290406b9..d8cb340d4 100644 --- a/R/AdditionTree.R +++ b/R/AdditionTree.R @@ -114,24 +114,31 @@ AdditionTree <- function (dataset, concavity = Inf, constraint, sequence) { tree } + .ConstraintConstrains <- function(constraint) { if (length(constraint[[1]]) < 1) { FALSE } else { - cont <- `mode<-`(attr(constraint, "contrast"), "logical") - nLevel <- dim(cont)[[1]] - # Could be > 2× more efficient using lower.tri - exclude <- vapply(seq_len(nLevel), function(i) { - colSums(apply(cont, 1, `&`, cont[i, ])) == 0 - }, logical(nLevel)) - - # TODO Validate; passes existing tests, but these do not include all - # edge cases, e.g. 02 03 1 1 - splits <- exclude * tabulate(unlist(constraint), nLevel) - any(splits[lower.tri(splits)] > 1 & t(splits)[lower.tri(splits)] > 1) + contrast <- attr(constraint, "contrast") + if (dim(contrast)[[2]] < 2) { + FALSE + } else { + cont <- `mode<-`(contrast, "logical") + nLevel <- dim(contrast)[[1]] + # Could be > 2× more efficient using lower.tri + exclude <- vapply(seq_len(nLevel), function(i) { + colSums(apply(cont, 1, `&`, cont[i, ])) == 0 + }, logical(nLevel)) + + # TODO Validate; passes existing tests, but these do not include all + # edge cases, e.g. 02 03 1 1 + splits <- exclude * tabulate(unlist(constraint), nLevel) + any(splits[lower.tri(splits)] > 1 & t(splits)[lower.tri(splits)] > 1) + } } } + .Recompress <- function(dataset) { MatrixToPhyDat(PhyDatToMatrix(dataset)) } diff --git a/tests/testthat/test-AdditionTree.R b/tests/testthat/test-AdditionTree.R index 91b863cbe..395e472cf 100644 --- a/tests/testthat/test-AdditionTree.R +++ b/tests/testthat/test-AdditionTree.R @@ -22,6 +22,16 @@ test_that("Addition tree is more parsimonious", { test_that(".ConstraintConstrains() succeeds", { expect_false(.ConstraintConstrains(NULL)) + + # Single level + expect_false(.ConstraintConstrains( + structure(list(A = 1L, B = 2L, C = 2L, D = 2L), weight = 1L, nr = 1L, + nc = 1L, index = 1L, levels = 0, allLevels = c("0", "?"), + type = "USER", contrast = + structure(c(1, 1), dim = 2:1, dimnames = list(NULL, 0)), + class = "phyDat") + )) + expect_false(.ConstraintConstrains( structure(list(A = 1L, B = 2L, C = 1L, D = 1L, E = 3L), weight = 1L, nr = 1L, nc = 2L, index = 1L, levels = 0:1, From 3f7971941917930749f3f5117b15b42e107fd276 Mon Sep 17 00:00:00 2001 From: RevBayes analysis <1695515+ms609@users.noreply.github.com> Date: Wed, 5 Feb 2025 18:57:03 +0000 Subject: [PATCH 6/7] link format --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index a9a91ffd6..333889154 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,7 @@ # TreeSearch 1.5.1.9003 (2025-02) - Improve support for constraints in `AdditionTree()` - (#173)[https://github.com/ms609/TreeSearch/issues/173] + [#173](https://github.com/ms609/TreeSearch/issues/173) # TreeSearch 1.5.1.9002 (2025-01) From eb1422feda30278a1f4747ae9ef21192c0d3947f Mon Sep 17 00:00:00 2001 From: RevBayes analysis <1695515+ms609@users.noreply.github.com> Date: Wed, 5 Feb 2025 19:02:49 +0000 Subject: [PATCH 7/7] +pandoc for pandoc-citeproc --- .github/workflows/coverage.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 8c68f5464..e02d360ca 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -54,6 +54,9 @@ jobs: needs: | coverage + - name: Set up pandoc + uses: r-lib/actions/setup-pandoc@v2 + - name: Code coverage run: | covr::package_coverage(quiet = FALSE) # https://github.com/r-lib/covr/issues/252