From 67ee0fb0dc9c881070ea3277f188ab46971b2855 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Thu, 29 Oct 2026 06:55:07 +0000 Subject: [PATCH 1/3] CI: unblock R 4.1 and benchmark legs The 7-leaf NNIDist() bounds test (#222) called NNIDist() on all 945 trees: 446,040 pairs through CompareAll()'s R loop, c. 80 s locally and over an hour on the R 4.1 leg, which then hit its 60-minute limit. The test now calls cpp_nni_distance() on pre-ordered edges (c. 7 s); the 6-leaf case still checks that NNIDist() agrees. benchmark and rev-dep-check ran on ubuntu-latest, now 26.04, where setup-r apt-installs ~175 packages and timed out at 15 minutes; their RSPM serves noble binaries anyway, so pin ubuntu-24.04. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/R-CMD-check.yml | 6 ++++-- tests/testthat/test-tree_distance_nni.R | 23 ++++++++++++++++++----- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/.github/workflows/R-CMD-check.yml b/.github/workflows/R-CMD-check.yml index bbdc2526..03024173 100644 --- a/.github/workflows/R-CMD-check.yml +++ b/.github/workflows/R-CMD-check.yml @@ -338,7 +338,9 @@ jobs: !cancelled() && needs.sense-check.result == 'success' && contains(fromJSON('["success", "skipped"]'), needs.codemeta.result) && (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch') - runs-on: ubuntu-latest + # Pinned: ubuntu-latest is now 26.04, on which setup-r apt-installs ~175 + # packages and has timed out; RSPM below serves noble (24.04) binaries. + runs-on: ubuntu-24.04 # A hung setup or a wedged benchmark otherwise runs to the 6-hour default, # holding a runner the whole time. timeout-minutes: 45 @@ -672,7 +674,7 @@ jobs: # and codemeta is skipped outside pull requests, so without this the # backstop never ran on push, schedule or workflow_dispatch. if: ${{ !cancelled() && needs.core.result == 'success' }} - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 # As benchmark timeout-minutes: 120 # revdepchecks are genuinely long name: revdepcheck (${{ matrix.config.deps }}) diff --git a/tests/testthat/test-tree_distance_nni.R b/tests/testthat/test-tree_distance_nni.R index 09ec2f7d..b33c69d2 100644 --- a/tests/testthat/test-tree_distance_nni.R +++ b/tests/testthat/test-tree_distance_nni.R @@ -186,13 +186,26 @@ test_that("NNIDist() lower bounds are valid", { testthat::expect_equal(max(exact), NNIDiameter(nTip)[, "exact"], ignore_attr = TRUE) - bounds <- lapply(NNIDist(trees), as.matrix) + # Bounds for each pair, in the order of exact[lower.tri(exact)]. + # Calling the C++ kernel directly avoids NNIDist()'s per-pair R overhead, + # which makes 7 leaves (446,040 pairs) take minutes, and an hour on R 4.1; + # tip labels are already in a common order. + pairs <- which(lower.tri(exact), arr.ind = TRUE) + edges <- lapply(trees, function(tr) Postorder(tr[["edge"]])) + bounds <- vapply(seq_len(nrow(pairs)), function(k) { + cpp_nni_distance(edges[[pairs[k, 1]]], edges[[pairs[k, 2]]], nTip) + }, integer(6)) + if (nTip == 6) { + viaR <- vapply(NNIDist(trees), function(d) as.matrix(d)[pairs], + double(nrow(pairs))) + testthat::expect_equal(t(bounds), viaR, ignore_attr = TRUE) + testthat::expect_equal(colnames(viaR), rownames(bounds)) + } exact <- exact[lower.tri(exact)] - .Bound <- function(field) bounds[[field]][lower.tri(bounds[[field]])] - testthat::expect_true(all(.Bound("lower") <= exact)) + testthat::expect_true(all(bounds["lower", ] <= exact)) for (upper in c("tight_upper", "best_upper", "loose_upper", "fack_upper", "li_upper")) { - testthat::expect_true(all(.Bound(upper) >= exact)) + testthat::expect_true(all(bounds[upper, ] >= exact)) } } @@ -201,7 +214,7 @@ test_that("NNIDist() bounds bracket exact distances: 6 leaves", { }) test_that("NNIDist() bounds bracket exact distances: 7 leaves", { - skip_on_cran() # c. 70 s + skip_on_cran() # c. 7 s .ExpectBoundsBracketExact(7) }) From bb046d216b7ae261747c6b46d86727035a2e4b91 Mon Sep 17 00:00:00 2001 From: ms609-agent <313734811+ms609-agent@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:16:04 +0000 Subject: [PATCH 2/3] CompareAll(): index a plain list, not a pair-length multiPhylo On R 4.1, ape's [[.multiPhylo (class(x) <- NULL) copies the whole list on each access. CompareAll() built two multiPhylo objects with one element per pair and indexed each once per pair, so its time grew with the square of the number of pairs: 446,040 pairs of 7-leaf trees took over 50 minutes on the R 4.1 CI leg, against c. 80 s on R 4.6, where the access does not copy. It now indexes a plain list of the trees by pair indices. Co-Authored-By: Claude Opus 5.5 --- NEWS.md | 4 +++ R/tree_distance_utilities.R | 21 ++++++++------ tests/testthat/_problems/test-parallel-19.R | 29 ++++++++++++++++++++ tests/testthat/testthat-problems.rds | Bin 0 -> 4400 bytes 4 files changed, 45 insertions(+), 9 deletions(-) create mode 100644 tests/testthat/_problems/test-parallel-19.R create mode 100644 tests/testthat/testthat-problems.rds diff --git a/NEWS.md b/NEWS.md index df06e482..718c7c07 100644 --- a/NEWS.md +++ b/NEWS.md @@ -35,6 +35,10 @@ ## Performance +- `CompareAll()` no longer takes time quadratic in the number of pairs on + older versions of R (e.g. 4.1), where each element access to a + `multiPhylo` object copied the whole list. + - Trees with different leaf sets are pruned to their shared leaves in C++, without constructing pruned trees in R: `TreeDistance()` on 150 gene trees is c. 15× faster, with lower memory use (#123). diff --git a/R/tree_distance_utilities.R b/R/tree_distance_utilities.R index 518b7bba..4cb4976d 100644 --- a/R/tree_distance_utilities.R +++ b/R/tree_distance_utilities.R @@ -535,28 +535,31 @@ Ntropy <- function(...) { CompareAll <- function(x, Func, FUN.VALUE = Func(x[[1]], x[[1]], ...), ...) { nTree <- length(x) + labels <- names(x) countUp <- seq_len(nTree - 1) - i <- x[rep(countUp, rev(countUp))] - j <- x[unlist(sapply(countUp, function(n) n + seq_len(nTree - n)))] - + i <- rep(countUp, rev(countUp)) + j <- unlist(lapply(countUp, function(n) n + seq_len(nTree - n))) + # A plain list: on R 4.1, each `[[` on a multiPhylo copies the whole list + x <- lapply(seq_len(nTree), function(k) x[[k]]) + cluster <- getOption("TreeDist-cluster") ret <- if (is.null(cluster)) { cli_progress_bar("Comparing", total = length(i)) vapply(seq_along(i), function(k) { cli_progress_update(1, .envir = parent.frame(2)) - Func(i[[k]], j[[k]], ...) + Func(x[[i[k]]], x[[j[k]]], ...) }, FUN.VALUE) } else { - do.call("cbind", - parLapply(cluster, seq_along(i), - function(k, Func) Func(i[[k]], j[[k]]), + do.call("cbind", + parLapply(cluster, seq_along(i), + function(k, Func) Func(x[[i[k]]], x[[j[k]]]), Func = Func)) } - + .WrapReturn <- function(dists) { structure(dists, Size = nTree, - Labels = names(x), + Labels = labels, Diag = FALSE, Upper = TRUE, class = "dist") diff --git a/tests/testthat/_problems/test-parallel-19.R b/tests/testthat/_problems/test-parallel-19.R new file mode 100644 index 00000000..37e11cd1 --- /dev/null +++ b/tests/testthat/_problems/test-parallel-19.R @@ -0,0 +1,29 @@ +# Extracted from test-parallel.R:19 + +# setup ------------------------------------------------------------------------ +library(testthat) +test_env <- simulate_test_env(package = "TreeDist", path = "..") +attach(test_env, warn.conflicts = FALSE) + +# test ------------------------------------------------------------------------- +library("TreeTools") +trees <- as.phylo(0:20, 20) +suppressMessages({ + expect_equal(options("TreeDist-cluster"), StartParallel(2)) + cl <- getOption("TreeDist-cluster") + + parallel <- ClusteringInfoDistance(trees) + SetParallel(NULL) + expect_null(GetParallel()) + expect_equal(ClusteringInfoDistance(trees), parallel) + + expect_equal(options("TreeDist-cluster"), SetParallel(cl)) + expect_equal(cl, getOption("TreeDist-cluster")) + expect_equal(cl, GetParallel()) + parallel <- CompareAll(trees, NNIDist) + SetParallel(NULL) + expect_equal(CompareAll(trees, NNIDist), parallel) + expect_false(StopParallel()) + SetParallel(cl) + expect_true(StopParallel()) + }) diff --git a/tests/testthat/testthat-problems.rds b/tests/testthat/testthat-problems.rds new file mode 100644 index 0000000000000000000000000000000000000000..47bbda638a15b888ebfcce0bb55321c0e999cdb9 GIT binary patch literal 4400 zcmW-hcRUpSAIHO2LS)OxRm#jxwo{TlN>;{|%#h7x+)<&-?Cp#=BhDTPb8ccFEx56~>85=a6dMmUYlVDD z_a0~z-v0i+R?40w)f6HuT4)yjYYMJ$vg0zKQ{T_TPph`kP0F`yZ{U^1Ls9Wc>mu8YGcxU~4CRr7eI`vTLh(iLrm!WyHj8 zW8y?I(A;C@-j>6R5KAt>_*+G-L+{;|yzJeAbG4-g>n>Qyrkk0$O^q@Yd!4rm^Tjfl z3lr~kFiU3x@(XhtGI8J?-dM$w^jQm8n5NbWAM#eAZpV)0wWbBE>{MKU)>Fi{WV!UF z9Ibh^jA^!&5Z#XZw;;cGL0KauGa?dg`oE18H(WP_2M>w@xIEN5roWQ!`WqFS-=f^B zDEwW=XGhnP$aew9V@&WcBvfAHAFl5#UUtnKeaXQ*XUT=skHyO}qwHhxl2tq5E?{57 zxo}r7$dI5dfNHyO>;xpcVC&KrN>z{M#>SpmeleF!EEgZH6A4sLZ@C}6h1=(V1eqHy z>X)}_2*}jpO1l(M&HV|71IJzEjv)bi!P&neW;S+`oOcv-5>X#qr*?HJ^GAgqEl^;F<*=7SHsa{-oFE3-mq;5G`_!_o^A0n% z2s(b1aj#8>8CArt!;HkgYrj`_UlKvkuVJavC}eo7YjULa%CvGczgpwgXkXHndXRtF z=bdq>mnK+8`@W=g+I^`*jCfr?un_;ZKj6T^VcVcimtP~89U*U*h2uvFHMT!!>|oz50hkMs>>uwY%%p2gMl7`>-{YPa^Y z{?g08Lo%g=6ND=pzuI#M{_OD9Of|9dv-MO(A4SzU@3Mgtm)}?qnV#BY_m4h>uDvhF z91b2SPz(HAgV^&^gt~;32Fj*uc_9=uQhGoQ{o9G+(@k2AIPUs_g1lr`Ovq}D(kGYi1GaUu*`GyX~f`@jv0mG z-=4OkX=M~Kc9-Xe_tevJId_+Z5!V@TyOZ9smwq6B-3h!6_^~RvZ_u3S;Gv47pDx*$St=OQc)d%{13@18
sWEniAFrX_)^aep>M7u*;v?55sjIN<^_Vgjc{ z*G#=?Y_AkRx9@4RV)FO7pO zwDPs{M~2FkZ33+nL*tb8-xlaw-x(->K^*XG?|wd^v`}Uy!G3ne1VoZF^3NnMu^)20 zzxLSTLTGX#`z$vm-I$cYCiiE|r|L3qpRQDu*@1JjC%k{X-Pou4GO@$H$iS&yEUs2f zaIr3z%MaGX-bYk$pG0VPv#?hCgRj0`ySsetz5cd|Ks1~Oyes5iUx(s5acc&7sZZhH$SY&fh6?GsWqHO{q6O{mffqKS1eY~h`S6wec$<4im`}{Rfv=!T77~X z=tx_j^8BBnc{So#%nxlO$OyMnD_iu{vKRYuV5L#@4D8i!r;PuBSSQQ7(<#9w=plmJ z`3&@dj?lxo@2V^2CUok&;6`-nhK+*kw(hi0ew0Z}5#e4N?@EmT{~3u5j`)CLJ`r>S z?YG$Tgf&80dsLLnD1iyrA<6TD`;+WjxzI7D5Q+V_^Ir%;m*HnkmZ|e>7w0{BTIH~z z&!~2<)xX#XI+1qhHg#VyEKQ=i{EitJi?)vxB1M;iB6nhXFajf&Iqo}sBjXwVq!7dU z2dL*pBUv<0<}v%vmTPh;_R;Lc$oKw@RE6Rm7R@PO_rBMMHo>_AD2E5fipFL5VW;UE zanc(Wsz%P4e9gRBe-%{Z*=fjfxE&>ho~;ZC^RK3va??Bl+)g7CEvbC2JCE1dDn!ZW zmNH-|(&S6>WM*qLC6W7#`@u8nZw%q+8MWpPvH>wp=*O7TCzT;t)RGX+WMKv)NrDTs zV8iPJ;^|3Tcs-~7iBZ2Km0QQ12iyp7KRv$;rZ$mawk|g}*o zLyQvT&a#FrIZ%iHAu8Q}5YNa~v<-TEJU_5+!*j59%A|n4*3;Q$d3K*7LLYW{B{_3RH;XMw%NWGEZtF`nNYV;7O z4FwDC=?608;U7^@=l3rTZVA9oBd@IM(FP3cp^eWzm*RU)D}m^fm`?Gf*Fw+;bkBuo z1Rzn{TX=gNObIfH=C)0lmRP}8sL%uuvxyy1oduzP>=%ANsy{|C?b8p23&4+2h%f0> zex4J&>nVRjPTnH7#Ox zE`%*W8+s}*DlIYpMRf(gCa5_D)mwz6pb?FyIG2JyNBk!9$;j-V)NCC@2{MHS3?gdI zVqt*lWrF5c8$oZM?RgbrAEN%f`2OqSXIdAbU0*g;x>cvBwGV#-kI7r6+t7nbxb}mS z6NXWHiM9JDC!(Vy8nDoWjh@)Hq6#vZdEF-iK?{e+{7WEzLnw9P)TH;wra06B7408S z^^M*8Zr`6y3C)3s63u$H=Ales{41T8;bf{V4RO2_8Si|0WJ~#cS)4?Dw2V$zod0&n zR;KMu>&3hdpnir6P9`(4Z_#L^(oJyh9(HZ!G_FVszz++t3u7pW)lH#P-~GS*J{-j@ zKgIeLAPuq1lM)0Mbo_>CzXH0Hoe`YA=w%y`FCZRakP18`8?SF4+C_(bnIfy7>tMNE z8X*vH2hHB?AYE@tgC)tt9Luf+5nE3sD6;GGi!=9lmZ+75|T^NAGeFf&DSqmM3XV$4n0J@y5|4uEO zFseVzH9>+_w0FQnTx!!vxQ-_1PaLr&F+H-~=6o`(Tqs}yp_@#>>;V1bF3!XUr2B8e zNHheY(i3f0Q{+i)bXV@Zl3tqBenU$d_^}d@4`43LM_);_B!ZB?7PEVwwx(otgiukGEe(<#M8 zMbqg;gyJR`e$Rc)YKXeVMkUdWbBhEWl9&oC+Y89R6!%m(u%brJ>k!A9Qf3?@&; ziyN9B!wYn&o$CY3w?i1NIkD{XpR5FG2mUfjh)a29pJ1>40IwA?{e7%K!|dWE5rCk8 zh>#|DUqWD3WGXo>^H)snt**o?S`h5-h3|xQ|2BvGY6m}3IZkELxTabdSQjv}V22x< z#;aNy)FS>VOkigGHiZwF*KD_a*Jh&jA+j3yINA^=%1K{UkY2uB!O0jH5XzsB21fmp+O6=8r zkI~kDK`guYxXprs$@Nr|AwUl>iB9lGs}$M_E{q7MG^rtWt`;wL*K2w{fHpFFUc>R!AIS^L z&&*q~*_u4vNNQK{k-YUy?4(bBP`Iurdj>r5`JVe{Uf4YSh|)pup6R%7OREHkC^Z=u znRP>&ZEtS(iKdeZKGU-Zo9+8?iR%eO%Qt zh!7HF^B-euRVmi!xZ)MsZcp&NT)R|Z`RjZ5{6Mdz7vob$!&{EQte7jDamqtC_blHm z@Ti}q+J-5*x_UnKgOy7aA*{_cnk)wXjQ4u0?>IJS8(e*<8s3!o{GMXT2U*h!_(Iyy z^ZTi<-I}R)Ux(ivB=~8E`%6U9^x(5G86D&8G$x*HjuGRM+{&@PQWTnXTN~q4K&mcI z3!e(ue{p%vC)N#4_u!d*EKZFxkiUiumL!3ckr8@OG|}MHo%fIm)LZ9L1f+B%97_2c zR(NtcMQZ07n|=Hqnv_)Zz60|+Yx(J-o1ylL#@r5e;&Xh=>1}c@3`W)2DOxKL(wOB4 zoqHG>G+mrlm+vi=rOlR|w5E|^dQ&_i?J4ASHD})27I*#YTV&CsnxP(<^Ro<0z$DW} zFPzn2HmjK}j>kGPBKjmeK3s_CT+`Bdn8VGvI$&ge`d? Date: Thu, 29 Oct 2026 09:22:05 +0000 Subject: [PATCH 3/3] CI: run benchmark and revdep on Ubuntu 26.04 Use resolute RSPM binaries rather than noble ones on a 26.04 runner. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/R-CMD-check.yml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/R-CMD-check.yml b/.github/workflows/R-CMD-check.yml index 03024173..fd5fde60 100644 --- a/.github/workflows/R-CMD-check.yml +++ b/.github/workflows/R-CMD-check.yml @@ -338,9 +338,7 @@ jobs: !cancelled() && needs.sense-check.result == 'success' && contains(fromJSON('["success", "skipped"]'), needs.codemeta.result) && (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch') - # Pinned: ubuntu-latest is now 26.04, on which setup-r apt-installs ~175 - # packages and has timed out; RSPM below serves noble (24.04) binaries. - runs-on: ubuntu-24.04 + runs-on: ubuntu-26.04 # RSPM below must name the same release # A hung setup or a wedged benchmark otherwise runs to the 6-hour default, # holding a runner the whole time. timeout-minutes: 45 @@ -356,7 +354,7 @@ jobs: env: GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} - RSPM: "https://packagemanager.posit.co/cran/__linux__/noble/latest" + RSPM: "https://packagemanager.posit.co/cran/__linux__/resolute/latest" _R_CHECK_BUILD_VIGNETTES_: false _R_CHECK_CRAN_INCOMING_: false _R_CHECK_FORCE_SUGGESTS_: false @@ -674,7 +672,7 @@ jobs: # and codemeta is skipped outside pull requests, so without this the # backstop never ran on push, schedule or workflow_dispatch. if: ${{ !cancelled() && needs.core.result == 'success' }} - runs-on: ubuntu-24.04 # As benchmark + runs-on: ubuntu-26.04 timeout-minutes: 120 # revdepchecks are genuinely long name: revdepcheck (${{ matrix.config.deps }})