C++ search - #210
Draft
ms609 wants to merge 1064 commits into
Draft
Conversation
ms609
marked this pull request as draft
March 25, 2026 14:21
ms609
added a commit
that referenced
this pull request
Mar 28, 2026
ms609
added a commit
that referenced
this pull request
Mar 28, 2026
ms609
added a commit
that referenced
this pull request
May 18, 2026
In R CMD check, R runs as a non-interactive subprocess with captured stdout. R_FlushConsole() calls fflush() on that pipe; when the buffer fills the call blocks indefinitely, causing the 6 h GHA timeout seen on every ubuntu runner for PR #210. Gate the \r-overwrite progress line and the flush behind R_Interactive (FALSE in batch/check contexts). Interactive sessions are unchanged. At verbosity >= 2 in batch mode, emit plain \n-terminated lines so diagnostic logs still carry progress detail without the flush risk. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ms609
added a commit
that referenced
this pull request
May 18, 2026
In R CMD check, R runs as a non-interactive subprocess with captured stdout. R_FlushConsole() calls fflush() on that pipe; when the buffer fills the call blocks indefinitely, causing the 6 h GHA timeout seen on every ubuntu runner for PR #210. Gate the \r-overwrite progress line and the flush behind R_Interactive (FALSE in batch/check contexts). Interactive sessions are unchanged. At verbosity >= 2 in batch mode, emit plain \n-terminated lines so diagnostic logs still carry progress detail without the flush risk. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 19, 2026
…nytime study
The prior commit's "no optimum-score regression ... +4% ... passes the mission
gate" was based on Part B's 4 non-discriminating benchmarks and is FALSIFIED by a
powered anytime study (array 17728529: 20 MorphoBank training matrices, 65-120
tips, base vs drift2_ws5, 10 seeds, 180s, time-to-optimum via progress callback):
- reach-rate DOWN on 7/18 discriminating datasets (drift is per-replicate
overhead -> fewer reps in a fixed budget -> reaches optimum less reliably);
- median score WORSE on 4/20 (+0.5..+2.5), better on 3, tied on 13;
- time-to-target median ratio 0.97 (neutral) but tails to 2.4x.
Per user direction, thorough = drift2_ws5 is RETAINED (the two-island completeness
win is wanted) but reframed honestly as a higher-effort, slower tier that needs a
larger replicate budget to converge. NEWS + preset comment corrected to state the
cost, not claim a free win. Effort-ceiling tuning (maxReplicates) to follow from a
budget-escalation cost study.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Empirically verified: fresh build of 0daea13^ (parent of the collapse-dedup fix) reproduces the reported "30 trees / n_topologies=30" on the finding's own repro dataset; fresh build of current HEAD returns 1. The finder cited ts_tbr.cpp:2345,2377, which are that commit's pre-fix line numbers, so the trace read a stale checkout/lib rather than stale source. T-326's regression ask is already covered by test-MaximizeParsimony-features.R:402-421 (added in the same commit; re-ran green). Rows removed from to-do.md/findings.md (net-zero diff, since the round's own additions there were never committed); closure recorded in completed-tasks.md and dev/red-team/log.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d_alloc (T-327)
impose_one_pass() captured `best_node` and the move-root lists once, then
applied a batch of topology_spr() moves. topology_spr relocates the node id
`best_node` when a move root's parent IS best_node (it detaches that internal
node and reuses it at the graft point). The stale reference then let a later
graft target land adjacent to (parent/sibling self-loop) or inside the moved
subtree, producing a cyclic / double-parented tree. The next unguarded DFS
(collect_edges_*, compute_node_tips, build_postorder) walked the cycle and grew
its stack without bound -> std::bad_alloc, failing constrained search on valid
input. Opt-in trigger: a topological constraint plus nniPerturbCycles > 0.
This confirms and closes the long-open stale-best_node hypothesis (T-213 note):
a 40-seed x 5-constraint sweep crashed 66/200 configs on the pre-fix build.
Fix (three parts):
* ts_constraint.cpp: re-anchor best_node (min-symmetric-difference rescan)
before every move, not just between loops -- it can go stale mid-loop too.
* ts_constraint.cpp: snapshot topology -> apply move -> validate that the
result is still an acyclic tree (postorder visits every internal node
exactly once) -> revert the move if not. Complete without enumerating
topology_spr's corner cases; a mere "target inside moved subtree" guard is
insufficient (it misses the parent/sibling self-loop edges).
* ts_tree.cpp: size-cap the two-pass build_postorder at n_internal (no throw)
so the validation terminates instead of OOM-ing, and as defence-in-depth on
the fuse / map_constraint_nodes path.
Regression test (test-ts-impose-constraint.R): "T-327: impose_one_pass survives
stale best_node" -- seeds 22/23/24 x 2 constraints, nThreads=1L so set.seed
deterministically drives the C++ search RNG (each pair crashed pre-fix, all pass
post-fix). Verified: 66->0 crashes; impose-constraint / constraint-small /
constraint-multi + 12 quarantined _problems scripts + core SPR/NNI/TBR/ratchet
suites all green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI on 91918d1 was green on all 10851 tests (FAIL 0); the only R CMD check ERROR was the spelling wordlist lacking terms introduced by the merged docs (PAUP, and the deduplicate verb forms alongside the existing deduplicated/deduplication). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
savePlotZip zipped lastFile("excel") and the other cached inputs unconditionally. When no Excel was uploaded, ExcelFileName(0) resolves to "excelFile-00.xlsx" - a path that was never created - and zip::zip(mode="cherry-pick") aborts ("Some files do not exist"), so the "R script" download failed for every non-Excel session (package datasets, .nex, TNT files). Guard each cached input with if(state$xFiles), mirroring the sibling saveZip handler.
Also fix a tree-file count skew: stashTrees() bumps r$treeFiles and writes a fresh treeFile-NN at download time, but the plot log referenced the plot-time treeFile-(NN-1) (cached in RCode, whose key excludes treeFiles), so the zipped script read a tree file absent from its own zip. Rewrite the treeFile reference to the file actually cherry-picked (mirrors the testmode branch). The real download branch is not covered by the shinytest snapshots.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Clearing the nTree numericInput sends NA; the is.null/length==0 guard missed it, so `if (n > length(r$allTrees))` evaluated `if (NA)` ("missing value where TRUE/FALSE needed") inside the observe(FetchNTree()) observer. Add `|| is.na(n)` to the early-return guard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Plumbs the existing SectorParams::rss_picks_per_round through the driven search path (SearchControl rssPicks -> DrivenParams -> SectorParams), so the number of sector picks BETWEEN global-TBR rounds is tunable (Goloboff 1999 sequential replacements; TNT ~20-25). Default 0 = auto (~5) => byte-identical default path. No algorithm change; exposes an already-present field. Adds the config-first mission-gate harness (dev/benchmarks/missiongate/): shared TNT T0 (12 cells) + a wall-vs-escape budget grid (baseline band vs selectem-config: large clade + sub-clade collapse + rasStarts=3 + rssPicks), replicated over sectorial RNG seeds, with a ratchet_ref yardstick. Answers: does the selectem sectorial escape the frozen T0 CHEAPER than the ratchet it would displace, or only at high budget (=> mission-dead)? Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ts_collapse_pool's reroot_at_tip/build_postorder assume every internal node has exactly 2 children (true of the engine's own pool, always binary). A caller reaching this internal function with a hand-built multifurcating edge matrix (bypassing MaximizeParsimony()'s own RootTree/MakeTreeBinary normalization) hung the per-tree loop indefinitely instead of terminating. Validate child counts at the Rcpp boundary and Rcpp::stop with a clear message instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
When every character is parsimony-uninformative (total_words == 0), compute_collapsed_flags and _aggressive early-returned with all-zero flags, leaving trees fully binary with an inflated n_topologies instead of collapsing to the correct single star. Flag every internal, non-root edge (including root's children) as collapsible in that case.
…t non-binary trees (T-332) Closes T-332: ts_collapse_pool now validates every internal node has exactly 2 children before the C++ reroot/postorder walk, erroring cleanly (Rcpp::stop) instead of hanging on a hand-built multifurcating edge matrix.
The previous commit accidentally reverted this guard + its test via stray staged changes from a concurrent session. Re-applying the T-332 fix verbatim (commit 1b9b5dc) with no other changes.
compute_collapsed_flags[_aggressive] (src/ts_collapsed.cpp) decided
min-length-0 branches by iterating ONLY the standard ds.blocks[]. HSJ
stores a clade's support in ds.hierarchy_blocks and XFORM in
ds.sankoff_*, both zero-weighted out of the standard blocks (by
.NonHierarchyWeights) and dropped by simplify_patterns. So a clade
supported solely by a hierarchy/Sankoff character looked unsupported and
was contracted:
1. final collapse via ts_collapse_pool over-collapsed result trees
(e.g. HSJ MaximizeParsimony(collapse=TRUE) dropped Nnode 5 -> 3);
2. the MPT-enumeration dedup (ts_tbr.cpp collect_pool path, unguarded
unlike the incremental-rescore paths) merged HSJ/XFORM-distinct
MPTs during search (n_topologies collapsed to 1).
Fix: no-op the collapse flags (all-zero == nothing collapses) whenever
ds.scoring_mode is HSJ or XFORM. Applied at the KERNEL (both functions)
rather than per-call-site: the task enumerated only the ts_tbr.cpp sites
+ ts_collapse_pool, but ts_drift/ts_temper/ts_search/ts_driven/
ts_parallel also call these functions -- guarding the two kernels covers
every caller by construction and cannot be bypassed. Falling back to the
conservative flags (the existing NA remedy) is insufficient here: that
path is equally blind to hierarchy/Sankoff support.
Regression test (tests/testthat/test-ts-t330-collapse-hsj-xform.R):
direct ts_collapse_pool proves an HSJ-supported clade survives WITH
hsjConfig while the EW arm still collapses (guard is scoped, not global);
end-to-end HSJ + XFORM assert collapse=TRUE preserves resolution and MPT
count. Verified discriminating: 4 failures on unpatched code, all pass
patched. New C++ guard lines are 100% covr-covered; existing
collapsed/hsj/xform suites still green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lock lever Hamilton array 17755546 (4 datasets x 3 T0 seeds x 5 RNG reps). 0/4 datasets escape cheaper than a single ratchet start; ratchet Pareto-dominates on Wortley/ Zanol, and selectem ras=3 reaches reliable escape on Zhu/Giles only at 2-7x wall. Thread retires to the budget axis. See FINDINGS.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…led out Foreground the diagnostic result (ours now escapes to TNT sectsch targets best-of-15 on all 4, was 0) so the retirement reads as 'clue resolved', not an ROI gate on a TNT win. Lead the wall verdict on candidate-efficiency (machine-portable) not sub-second wall ratios. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…31/T-332 work # Conflicts: # src/ts_collapsed.cpp
…ecord Keeps the reusable rssPicks SearchControl knob (exposes existing SectorParams::rss_picks_per_round; default 0 = byte-identical) and the mission-gate harness+FINDINGS documenting the sectorial-escape thread's closure (mechanism resolved, wall-lever ruled out). See dev/benchmarks/missiongate/FINDINGS.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
test-ViewChars.R and test-Distribution.R were silently broken since the app was modularised: they drove consensus inputs by pre-modularisation unqualified ids (keepNTips, consP, outgroup, concordance ...), so every consensus interaction no-oped and the committed baselines had drifted stale (still citing the long-removed MorphyLib refs). Nothing caught this because these tests are not run in CI. Also fixed mapLines, which is a top-level input (ui.R) that had been wrongly namespaced treespace-. - Namespace every input to its real id (consensus-*, mapLines top-level); regenerate and certify the ViewChars/Distribution value baselines (verified the app now reaches each intended state). - Port the legacy tests/shinytest/*.R download-content coverage: add savePlotZip script snapshots via expect_download() at the same states, with a shared normalize_download() transform (setup.R) that is CRLF-safe and scrubs dates/versions/system lines. Teeth verified: a meaningful script change is detected; a volatile version change is scrubbed. - SearchLog: the saveZip session log embeds non-portable search results (trees[[N]], allTrees[1:M], nThreads); assert deterministic log-generation markers instead of a brittle full-content snapshot, and check saveNwk/saveNex structurally. This ports v1 intent in a CI-portable form. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The three legacy shinytest scripts (Distribution.R, SearchLog.R, ViewChars.R) plus their -expected snapshots and the shinytest.R runner are now fully superseded by their shinytest2 AppDriver counterparts under tests/testthat/ (interaction coverage, plus the download-content coverage ported in the previous commit). The v1 runner could no longer execute anyway: shinytest (v1) was dropped from DESCRIPTION Suggests some time ago (only shinytest2 remains), so `library(shinytest)` errored on the first line. codemeta.json still lists shinytest; that file is generated (see .github/workflows/codemeta.yml) and will drop the entry on its next regen from DESCRIPTION, so it is intentionally left untouched here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ARCH
Array 18080528, 300/300 COMPLETED. 30 native inapplicable matrices x 5 seeds x
tabuSize {100, 0}, unit of replication = MATRIX (n = 30). Raw cells in
dev/profiling/na-certify-gate-hamilton.csv.
At EQUAL REPLICATES the gate regresses floor attainment at both tabu levels
(tabu 100: -0.100, 3 better / 12 worse, p = 0.035; tabu 0: -0.140, 0/12,
p = 0.00049). By the stated rule it does not ship as a default, and the default
stays opt-in.
At EQUAL WALL the same gate wins decisively under the production preset:
B_gate_mw +0.167, 10 better / 1 worse, p = 0.012 -- while spending 10% LESS wall
than the arm it beats. Every matched-wall arm came in at 0.90-0.97x arm A's wall
(maxSeconds is polled at replicate boundaries, so a replicate that would overrun
never starts), so those wins are floors, not artefacts. Gains land where the
engine is weakest: Aguado2009 0.0 -> 1.0, Geisler2001 0.2 -> 1.0, Zhu2013
0.2 -> 1.0, Dikow2009 0.4 -> 1.0, Liljeblad2008 0.4 -> 1.0.
So the finding is NOT that certification is wasteful -- 16% of sweeps found a
real improver (1087 of 6957). It is that certification is OVER-SEARCH: its wall
buys more reach spent on replicates. Which regime production is in therefore
decides the default, and MaximizeParsimony defaults to maxSeconds = 0 with
maxReplicates/targetHits -- the replicate-bounded regime, where the gate loses.
Capturing the matched-wall win by default means gating AND raising the NA
replicate budget together: a recipe change, gated on its own panel, not a flag
flip here.
Firing is confirmed, so no arm's result is vacuous: at tabuSize = 100 B_gate
executed 0 sweeps against 5526 skipped -- under the shipped preset the gate
removes ALL certification.
Zanol2014, the hardest matrix in the corpus, is the honest counter-example:
B_gate_mw is -0.2 there at both tabu levels, so certification pays even at
matched wall. C_final_mw recovers it and is the only arm in the panel with zero
regressions anywhere (5 better / 0 worse at tabu 0) -- the arm for the hard tail
if a difficulty-conditioned default is ever built.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… not uniform Three corrections to the verdict as first written: * "Set maxSeconds and gate" is NOT the arm that won. B_gate_mw ran with maxReplicates = 1000 and completed a median 101-226 replicates on 23 of the 30 matrices at arm A's wall (Zhu2013 226, Zanol2014 173). The shipped cap of 96 binds on all 23, so that recommendation would deliver a replicate-capped run, not the measured arm. The recommendation is gating TOGETHER WITH a raised maxReplicates. C_final_mw is the exception -- it never exceeded 33 replicates, so the cap never binds for it; it buys less but works with today's budget. * The equal-replicate regression is uniform only at tabuSize = 0 (0 better / 12 worse). At tabuSize = 100 three matrices IMPROVED with certification removed, so the sign test reports a net, not a one-way mechanism. The verdict is unchanged (12 > 3, p = 0.035) but the prose no longer claims a direction the data do not show. * Zanol2014's per-block floor differs (1311 at tabu 0, 1312 at tabu 100), so its 0.2 in the two blocks is not the same achievement -- flagged, since it is the matrix singled out as the counter-example. C_final_mw's "zero regressions" softened to "no regression observed": 5/0 is p = 0.063 on five discordant matrices. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ules Panel 1 neutralised targetHits/stopPatience so the budget alone ended a run, and measured two budgets: 8 replicates (gate loses, p = 0.035) and matched wall (gate wins, +0.167, p = 0.012). Production is NEITHER. MaximizeParsimony defaults to maxSeconds = 0, maxReplicates = 96, targetHits = max(10, ntax/5), so a real run stops on hits-to-best. Nothing measured so far says where that regime falls, and it is the only regime that can justify changing a preset default. The mechanism at issue: targetHits counts hits against the CURRENT best, not the true optimum. A weaker gated search accumulates hits more slowly, so it runs MORE replicates and gets more chances -- but if it reliably plateaus one step above the optimum it accumulates targetHits hits on THAT score and stops early, confidently wrong. A local smoke on Longrich2010 shows the first effect live: B_gate needed 13 replicates to reach 10 hits where A_default needed 10, at under half the wall. Six arms: A_default / B_gate / C_final untouched; B_gate_reps pairs the gate with maxReplicates = 480, because panel 1's WINNING arm spent 101-226 replicates on 23 of 30 matrices and the shipped 96 cap would have bound on all 23 -- gating without raising the cap is not the arm that won. A_hits3 / B_gate_hits3 run at 3x the default hit target, which answers "does anything need adjusting as targetHits rises?" directly. Records reps and hitsToBest alongside score and wall: "where did it stop, and why" is the question, so a null on score with a large reps difference is a finding, not a wash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…upling Presets unchanged today: neither panel-1 budget is the shipped one. thorough keeps certification permanently (its objective is reach, and the hard tail is where certification still pays at matched wall). For default/auto the candidate is a PAIR -- TS_NA_NOCERTIFY plus a raised maxReplicates -- because gating alone is the arm that lost and the 96 cap is what stops the freed wall being spendable. targetHits needs no compensating adjustment and raising it helps, since more replicates is the regime the gate won in; it is already the shipped escalation signal (.IwRatchetDepth reads targetHits/defaultHits). But it counts hits against the CURRENT best, so it cannot rescue a search that plateaus one step high -- it just buys more confirmations of the same wrong score. maxReplicates is the knob that pays for gating; targetHits is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
map_constraint_nodes() accepted a constraint split only when the canonicalised (tip-0-excluded) side was a rooted clade. A split is an unrooted bipartition, displayed whenever EITHER side is a clade, so a valid tree in any other rooting mapped to -1 -- which regraft_violates_constraint() reads as "the tree already violates" and answers by rejecting every regraft. The tree is legal; the search simply cannot move. Its comment said this "shouldn't happen if the starting tree is valid", which stopped being true when 7685bf0 landed. Accept the split or its complement, recording which in a new ConstraintData::constraint_complement flag. That is exactly the rooting-agnostic displayed-split test, at the exact-match test's price: for a split displayed by edge (parent(v), v) with v != root the two sides are desc(v) and its complement, so some node's mask equals one side or the other -- both only at the root's own bipartition. The complement scan runs only after the split scan has missed, so every tree that mapped before maps to the identical node. regraft_violates_constraint() swaps MUST_INSIDE/MUST_OUTSIDE when the flag is set: with cn holding the complement, a clip whose tips are all outside the split must land inside cn. The boundary-edge exemption (below != cn) mirrors unchanged. The other eleven map_constraint_nodes() call sites need no edits -- all read constraint_node only as a < 0 predicate, and clip_zones works from tip masks alone, so it was already rooting-agnostic. Both builders size the new field; the per-worker ConstraintData copies are whole-struct, so polarity cannot invert in parallel mode (checked, then confirmed at 100/100 and 12/12 compliant with identical scores at 1 vs 2 threads). impose_one_pass()'s violated-split detection ran the same exact-match test, so a tree already displaying every constraint through its complement was "repaired" with up to n_tip/4+2 arbitrary SPR moves -- reached unconditionally from ts_nni_perturb.cpp after every perturbation cycle, so it degraded the score rather than merely wasting time. Detection now accepts either side; the repair still aims at the canonical side, which displays the split either way. Guard: test-ts-constraint-rooting.R presents one constraint-valid but suboptimal tree in all 22 rootings, TBR-only, through ts_driven_search() -- the entry that takes the rooting as given, as the internal producers do. MaximizeParsimony() normalises the rooting of a user tree and so cannot reach the defect at all. Verified sensitive against a clean pre-fix build: 16/22 rootings return the start tree unimproved before, 0/22 after; compliance 22/22 in BOTH builds, confirming the defect cost search progress and never legality. Wall cost none, measured interleaved over 4 alternating rounds: the six always-mapping rootings 0.0110s vs 0.0110s (n=24 pairs, median -0.7%, POST slower 9/24) and the default recipe 0.0400s vs 0.0398s (n=12 pairs, -0.8%, slower 5/12); no case >10% slower, scores identical throughout. Also repairs test-ts-wagner.R's "constrained sequential Wagner boundary edge" assertion, which was failing at 12a5866 identically on a pre-fix build -- so it was the test, not the engine. TreeTools' %in% is an S4 method over a non-generic base function and did not dispatch, answering FALSE for a tree that does display the split. Replaced with an explicit complement-aware displays_split() helper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Marks T-384 fixed by 4c66a55 and appends the resolution: the option taken and why (mask-or-complement is the rooting-agnostic displayed-split test, so option (c)'s correctness at option (a)'s price), the eleven call sites left untouched, the impose_one_pass() sibling defect, and why option (b) is dead (tbr_search re-roots the whole tree itself). Records three corrections to the row's own reachability claims, all measured with a probe-instrumented build: - MaximizeParsimony(tree =) cannot reach the defect: the R layer re-roots any start tree whose root's first child is not a tip, and Preorder() orders children by smallest descendant tip, so only tip 1 can occupy that slot. - The tip-0-at-root invariant is violated constantly (71-296 mapping calls per search) and that is NOT sufficient to trip the bug: what decides the mapping is whether the root POSITION lies on tip 0's side of the split, which is strictly weaker. - 0 complement-hits in ~90 searches / ~50k mapping calls across 2 matrices x 3 constraint shapes x 7 seeds plus 14 config arms, so the residual was latent -- one topology shape away, guarded by nothing but the dynamics -- not a live wall or reach loss. No recurrence of 4.7x is claimed and none was found. Plus the 16/22 -> 0/22 guard, sensitivity verified against a clean pre-fix build, and the interleaved null wall result. Also carries the T-364 three-arm battery prose left uncommitted in the working tree by the session that landed 128f732, rather than dropping it now that session has ended: the 1.43x figure reattributed (not retracted) to complement-without-reroot, on the finding that arm 1's violating set and arm 2's complement-rooted set are the same set; and the "every phase slower on a score-identical trajectory" diagnostic disqualified, its sign flipping with the replicate budget on identical code and an identical defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The first submission of the stopping-rules panel ran `git fetch && git reset --hard` inside the array task. 150 concurrent tasks raced on one index and 114 died on `.git/index.lock` (exit 2), leaving 36 of 150 cells. Panel 1's array had no git ops -- only its build job did -- so this was a regression against a working pattern. The lock failure is the lesser problem. `reset --hard` mutates the checkout that the surviving tasks are reading, so a cell that "succeeded" could have run a half-rewritten script. Provenance has to be established, not assumed: all 36 survivors printed `Git HEAD: 644b5b1`, and the 114 failures printed the stale b9bc14d from the panel-1 build, so the survivors are sound and reusable. Sync the repo ONCE from the login node before submitting and keep it PINNED for the life of the panel, so every cell provably runs the same code. The `Git HEAD:` line is the evidence and must be identical across all cells before they are pooled. Repo pinned at 644b5b1 (byte-identical cell script to HEAD); failures resubmitted as 18126933 with the same pin, so the two batches pool cleanly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One dial instead of several. `effort` says how much search to spend relative to the amount this dataset warrants: 0 (the default) accepts the automatic choice, 1 asks for one notch more, -1 one less. So a single call means "try harder than usual" whether the matrix has 20 taxa or 200, and a user never has to know which preset it would otherwise have got. `strategy` is removed rather than deprecated: it never shipped (2.0.0 is still unreleased; last release 1.8.0). Relative, not absolute, because the automatic choice is size-aware: `auto` picked `thorough` at 65-119 tips and `large` at >=120. An absolute ladder whose rung 2 was the literal `default` preset would silently downgrade exactly the datasets that need the most search. `effort = 0` therefore reproduces the previous behaviour on every size band, which is asserted directly. The rungs are the former presets, so this generalises an axis the package already had rather than inventing one -- `large` was only ever `presets$large <- presets$thorough` plus `maxReplicates = 500`, i.e. already "thorough at one notch more". Beyond it each notch doubles the replicate budget (1000, 2000, 4000, 8000) and raises the hit target with it. The budget LEADS and `targetHits` follows, because they bite on disjoint populations: `targetHits` ends a run early on easy datasets, but on hard ones it is never reached and `maxReplicates` binds first. Measured (array 18096945, equal weights): Zanol2014 used its full 96 replicates at hits-to-best = 1 against a target of 14, and tripling `targetHits` to 42 changed score, replicate count and wall not at all. A rung raising `targetHits` alone would do nothing on precisely the datasets someone turns effort up for. Under implied weights a raised hit target additionally deepens the ratchet, so it is raised alongside the budget from rung 5, not instead of it. Rung 8 is a stated ceiling with a message, not a silent clamp: `500 * 2^(rung-4)` overflows R's integer type around rung 26, and clamping there would make `effort = 40` quietly mean `effort = 26`. Same posture as `.iwRatchetMaxCycles`. The replicate cap now comes from `.RungSpec()` -- the one place a rung's budget is defined -- rather than a `switch()` on the preset name, so rung 4 cannot end up with two disagreeing sources. Preset NAMES are retained as internal menu labels, reachable only through the internal `.rung` argument, which also carries "none" (apply no preset at all). That is deliberately not user-facing: "no preset" must not be reachable by an accidentally-missing value propagating into `effort`, and it fails toward LESS search. Two tests and the preset smoke tests need it. Migrated: 36 call sites, 3 vignettes, the Shiny app (select box -> effort slider) and its shinytest2 expectations. The obsolete abbreviation-resolution and "Unknown strategy" tests are deleted -- a numeric offset has no names to abbreviate -- and replaced by validation, clamping and effort-0-equivalence tests. `strategy_diagnostics` is untouched: that is the adaptive starting-tree bandit, an unrelated concept that keeps its name. Two Rd files unrelated to this change are regenerated (they had drifted from their roxygen source). Suite: 11579 assertions pass. The one failure, test-ts-wagner.R:398, is the recorded `%in%`-on-Splits namespace trap in a file this change does not touch: it passes under a search-path-scoped runner and fails when the test env's parent is the namespace. Filed separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two guesses were stated as if they were findings. Both are corrected, and the remaining guess is now labelled as one. RUNG 8 HAD NO BASIS. It was justified as "the house style caps at the largest measured value", but that is not what it did: the largest measured budget is 500 replicates (rung 4). 8000 is no more measured than 16000. The comparison to .iwRatchetMaxCycles = 115 was also wrong in kind -- that caps ratchet DEPTH, which is not known to be free, whereas extra replicates are: raising the cap only appends later replicates and never delays an earlier improvement, so reach is monotone non-decreasing in maxReplicates and the only cost is wall, which is exactly what someone raising `effort` is asking to spend. There is therefore no level at which the ladder should refuse to go further. The ceiling is now 26 -- where 500 * 2^(rung - 4) stops fitting in R's integer type -- and says so. MIXING RATES WAS INCOHERENT. maxReplicates doubled while targetHits grew linearly. The two govern disjoint populations (targetHits ends easy runs; maxReplicates binds on hard ones), so a notch was 2x the work on hard datasets but only (k+1)/k on easy ones -- notches SHRANK as you climbed, on precisely the population targetHits controls. Both now double, so one notch means roughly twice the work whichever bound a dataset is under. WHAT IS STILL A GUESS, now said out loud in the docs: the doubling itself. What is measured is that reach was still climbing at 500 replicates with no knee (34-matrix 120-180t sweep, reach@96 = 0.68 -> reach@250 = 0.79). Nothing measures where that curve flattens or what shape the approach has, so rungs 5+ are an operating point -- "spend about twice as long again" -- not calibrated levels. A doubling grid over rungs 4-8 on the hard tail is what would replace the guess with a measurement. New tests pin the ceiling at exactly the representability limit (one rung lower would be arbitrary, one higher overflows) and that both knobs double in step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tests were already green (FAIL 0 across 11612); the coverage wave was skipped
because sense-check reported 1 ERROR, 1 WARNING and 2 NOTEs, all from the recent
`effort` work. This clears the two that gate the build, plus one long-standing
NOTE that was generating repeated man-page churn.
ERROR (spelling): `Zanol`. `inst/WORDLIST` already carried `ZANOL`, from
`R/data.R`'s all-caps citation, but not the mixed-case form that reached prose.
WARNING (undocumented argument): `.rung`, added to `MaximizeParsimony()` without
a `@param`. R CMD check requires every argument in a `\usage` section to be
documented even when it is deliberately internal, so it is now documented AS
internal -- what it pins, that `"none"` means no preset at all, that `NULL` is the
ordinary path, and that callers should prefer `effort`. This is the check
AGENTS.md names for a signature change (`devtools::check_man()`); running it would
have caught this before the push.
NOTE (empty \references): `expected_mi`'s roxygen carried
`@references \insertAllCited{}` while citing nothing, so Rd generation emitted an
empty `\references` section. Dropped at source in `src/expected_mi.cpp`. This
also stops `man/expected_mi.Rd` re-churning on every unrelated roxygen run.
NOT touched, and worth knowing:
- "Version contains large components (2.0.0.9999)" is self-inflicted by CI, not by
the repo. DESCRIPTION says `2.0.0`; the workflow's own "Temporarily bump package
version" step (.github/workflows/R-CMD-check.yml:77) appends `.9999` whenever the
version has exactly two dots. Fixing it means changing that step, which is a
release-process decision rather than a docs fix.
- The `Remotes` field and the `MaxMin` suggestion are flagged by the CRAN-incoming
check but are deliberate for a development package.
- `man/SiteConcordance.Rd`'s DOI 10.1201/9780367823412-8 now 404s upstream.
None of these three is a WARNING or an ERROR, so none of them gates the build.
Verified: `spell_check_package()` clean; `tools::checkDocFiles()` reports no
undocumented or mismatched arguments; the `man/expected_mi.Rd` diff is exactly the
three lines of the empty section and nothing else.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…hine's speed The coverage wave ran for the first time and immediately failed on my own parallel stopPatience test: FAIL 1 / PASS 11627 under `covr::codecov()`, against FAIL 0 / PASS 11628 in the same commit's ordinary check. So the test was not wrong about the code -- it was wrong about the environment. Mechanism, and it is the one I documented in the very commit that added the test (998891e): on the parallel path the dry spell is counted over replicates completed into the shared pool and evaluated only when the coordinating thread polls, every 200 ms. covr instrumentation makes each replicate far slower, so far fewer land per poll interval and `stopPatience = 3` fires genuinely earlier in the search -- 13.063 against 12.988. `expect_equal(short score, full score)` was therefore asserting a coincidence of timing, not a property of the rule. Stopping sooner is *allowed* to cost score; that is the entire trade-off `stopPatience` exists to offer, and it is what the shipped preset measurements quantify. Asserting equality contradicted the feature's own documentation. What the rule does guarantee is already asserted and untouched: the search stops before `maxReplicates`, and `perturb_stop` is set. The score check is now only that it is finite and not grossly worse, which holds at any speed. Third time this session a test of mine encoded an environment assumption -- after `as.Splits`/`%in%` dispatch and the ARM64 ordering chase. The pattern: an assertion that passes because of how fast, how attached, or how ordered the local environment happens to be, rather than because of what the code promises. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-374 Coordination-only (to-do.md + findings.md). No code. Three P1s (T-374, T-375, T-376) were sitting in findings.md with NO to-do.md row, and T-385 existed only in agent memory -- in neither file. The queue therefore read as "one open P3" (T-324) while four P1 correctness defects were live, which is also why the standing tasks were computing as P1 under to-do.md's own <3-open-tasks rule. T-385 is filed as the implementation ticket for the ALREADY-DECIDED T-374b policy, not as a new finding: T-374's row already records both of its symptoms, so a new number would have duplicated it -- the T-383 mistake recorded in the HSJ hub. Its detail is folded into T-374 instead. Also appended to T-374 a refinement that matters to whoever implements it: Q5 of the decision document treats BOTH symptoms as reporting defects fixed by making one rooting authoritative at the boundary. Measured against tip a8fbba8, that holds for the reported-vs-TreeLength gap but NOT for the MPT-set symptom -- pool membership is selected at MaximizeParsimony.R:1618 on search-time scores taken at differing rootings, so canonicalising the report path leaves 2 distinct scores among 32 "equally parsimonious" trees. Recorded as the open residue of T-374, explicitly out of scope for the reporting fix.
…as invalid Withdrawn same day, before any fix was written, so nothing was built on it. The claim (pushed in 7d4d255) was that the MPT-set symptom survives a canonicalised report path, evidenced by "2 distinct scores (183, 182) among 32 MPTs at a common tip-1 rooting". The measurement rooted each tree at its OWN tip.label[1], which after Renumber() is a DIFFERENT TAXON for different trees -- so it was a different rooting per tree, not a common one. Re-measured with the rooting taxon named explicitly (dev/red-team/heavy-tests/t385-diagnose-rooting.R): all 32 returned trees are already rooted at the kernel's tip 0 (32/32, root degree 2), and at a genuinely common rooting they all score 183 -- 32 distinct topologies, 0 disagreement. The pool IS self-consistent, so pool re-filtering is NOT required and n_topologies/collapse semantics do not need to move. That materially simplifies T-385. What survives is the P1 itself: reported 178 is not the score of the tree at the rooting it is returned at (183). And the objective is genuinely rooting-sensitive -- the same topology spans 178-183 over 8 rootings, inside the Sum nSec = 12 bound, with 178 attained at 2 of them. Fourth environment/measurement artefact this session (after %in%-on-Splits dispatch, ARM64 tip ordering, and covr timing). Same root cause each time: asserting on something incidental -- here "tip.label[1]" as a stand-in for a fixed taxon -- instead of naming the thing I meant. See [[loadall-is-not-rcmdcheck]] lesson 3.
Pre-fix reproduction against cpp-search tip a8fbba8, written before any fix so the failure is on record independently of the change that addresses it. The finding was recorded today but the tip has moved since (T-391 landed), and [[redteam-verify-against-current-tip]] says to re-verify rather than assume. All three symptoms reproduce on a 36-tip / 6-block / nSec=2 synthetic dataset (seed 1, search seed 11, maxReplicates 4): reported attr(res, "score") 178 TreeLength(res[[i]], ...) as returned 183 for all 32 trees -> gap 5 same topology, 8 rootings 178..183, spread 5 (bound Sum nSec = 12) 32 MPTs at a common tip-1 rooting 183 and 182 -> 2 distinct Note the reported 178 IS attained, by 2 of the 8 sampled rootings: the engine is not computing a wrong number, it is recording a score at one rooting and returning a topology rooted elsewhere (ts_collapse_pool's tip-0 canonicalisation being the trigger site named in T-374). The script reports rather than asserts, and exits 1 while the gap is open, so it doubles as the acceptance check for the fix.
Implements the ALREADY-DECIDED Option 3 of dev/plans/2026-07-29-t374b-xform-rooting-policy.md, whose three commits on cpp-search (7a18a4b et al.) turned out to be docs only -- no code implemented "make MaximizeParsimony's reported score and TreeLength() agree on one rooting". ## The defect XFORM's step matrix is asymmetric (gain = nSec + 1 against loss = 1), so a tree's length depends on where it is rooted -- unlike the symmetric criteria. MaximizeParsimony reported `result$best_score`, recorded mid-search at whatever rooting the replicate held, while ts_collapse_pool hands every tree back re-rooted on tip 0. So the reported number was not the length of the tree returned, and re-rooting a returned tree changed it again. Reproduced on tip a8fbba8 (36 tips / 6 blocks / nSec 2), pre-fix: reported 178 | TreeLength(returned) 183 on all 32 trees | gap 5 same topology over 8 rootings: 178..183, spread 5 (bound Sum nSec = 12) The reported number was not WRONG -- 178 is attained by 2 of those 8 rootings -- it was a value at a rooting the user never receives. ## The fix Canonicalise at both boundaries, on the dataset's first taxon (the tip-0 rooting ts_collapse_pool already imposes): - TreeLength(): root before scoring, in BOTH the single-tree and multiPhylo methods. The multiPhylo path previously rooted only trees that arrived unrooted, so an already-rooted tree kept its own rooting. Rooted by NAME with tips re-aligned afterwards, never by an R reroot on the edge matrix (na-validation-alignment-gotcha). - MaximizeParsimony(): rescore the returned pool through that same path and report the result -- |pool| evaluations. Post-fix the acceptance script is clean: gap 0, spread 0 across 8 rootings. Scores may therefore differ from previous versions and will not decrease; the value is now the length of the tree in hand, an upper bound on the rooting-free minimum exceeding it by at most Sum nSec (attained by 87-98% of rootings). Deliberately NOT done: min-over-rootings reporting. It is what the plan calls the better variant, but it reports a quantity the search never compared and costs (2n-3)x on the Sankoff term, so it stays Option 4. HSJ reporting is also untouched -- there rooting-invariance is REQUIRED by the method (two-state DP, T-374's open half), so canonicalising would convert a wrong objective into a stably-wrong one. Gated on XFORM alone, with that reason in the code. ## Scope boundary, and a warning where it bites Pool membership is still decided on search-time scores taken at differing rootings, so the returned trees need not share the canonical length. That is T-374's open residue, out of scope here -- but no longer silent: MaximizeParsimony now warns and reports the smallest. The warning immediately found a live instance in the existing suite (all-hierarchy data spans 7 to 9), which independently corroborates the earlier session's "4 of 6 trees do not share a score" observation. ## Tests Two regression tests, both verified to FAIL against the pre-fix tip built in a throwaway worktree (spread 2 not 0; reported 46 against min 48) and pass after: - The deterministic one carries its own anti-vacuity guard: it first ASSERTS via ts_sankoff_test -- untouched by this fix -- that the raw kernel really is rooting-sensitive on its 8-tip input, so it cannot pass by scoring rooting-insensitive data. - The end-to-end one asserts the actual contract (reported == min of the returned trees' canonical lengths), not the stronger "every tree shares it", which holds for its data but is not what the code promises. Also fixes the three false root-invariance comments the plan lists (ts_tbr.cpp:123 and :3152, ts_rcpp.cpp's collapse reroot, MaximizeParsimony.R's collapse note), and documents the behaviour in ?MaximizeParsimony, ?RecodeHierarchy, NEWS.md and vignettes/search-algorithm.Rmd. Suite green: xform, tree_length, hsj, t330, recode-hierarchy, resample-hierarchy, t306, prune-reinsert -- 0 failures.
… rename Not part of T-385 -- surfaced by running check_man() for it. 419168d changed the roxygen example from "one notch less / effort = -1" to "one notch more / effort = 1L" but man/MaximizeParsimony.Rd was never regenerated, so the shipped example contradicted its own source. Regenerated from the roxygen block, which is authoritative.
Coordination-only. The reporting half of T-374 is implemented on claude/t385-xform-report-agreement (stacked on PR #277): TreeLength() canonicalises the rooting in both its single-tree and multiPhylo methods, and MaximizeParsimony() rescores the returned pool through that path before reporting. Acceptance script: gap 5 -> 0, rooting spread 5 -> 0. Worth the row update on its own: the new "returned trees do not share a length" warning fired immediately on an EXISTING test -- all-hierarchy xform data returns a pool spanning 7 to 9. That independently corroborates T-374's original "4 of 6 trees do not share a score", which my own 36-tip attempt to reproduce had measured wrongly (retracted in 9296e87). The residue is real, reachable, and now visible at runtime instead of silent.
The acceptance script carried its own copy of MakeXformData() identical to the one the rooting diagnostic sources, so the two could silently drift. Sourced from the shared file instead. Acceptance script re-verified: exit 0, gap 0, spread 0 (checked WITHOUT a pipe -- the earlier 'EXIT: 0' was tail's status).
Three follow-ups, no behaviour change. - test-ts-xform.R's all-hierarchy test now expect_warning()s the "do not share a length" warning instead of emitting it into an otherwise-green suite. That matrix provably triggers T-374's residue (pool spans 7 to 9), so the warning is an expectation, not noise -- and if it ever stops firing, the residue has been fixed or masked and this test says so. Assign INSIDE expect_warning(): it returns the condition, not the expression's value, so `res <- expect_warning(...)` silently bound the warning object and broke the two assertions below it. - Both diagnostic scripts source() a path relative to the package root, so they only work when invoked from there. They now stop with that message instead of failing obscurely -- the same relative-path trap that made five tree_length tests "fail" earlier today when test_dir() changed the working directory out from under a relative lib.loc. - Recorded the real reason min-over-rootings reporting is declined. "Reports a quantity the search never compared" is the weaker half; the deciding objection is cost -- naively (2n-3)x on the Sankoff term, so a 4000-tip pool of 100 trees needs ~800k boundary evaluations. Affordably it needs an all-rootings up-down DP, which is Option 4's own piece of work.
All 8 CI jobs green on run 30662279563. The one ubuntu-24.04 (4.1) failure was transient -- it died in 'Set up R dependencies' with the Bioconductor 3.14 repos unreachable, so 'Check package' was skipped and no code was ever tested; green on re-run of that job alone.
T-385: report the x-transformation score of the tree actually returned
…f 30) Array 18126933, 150/150 cells at pin 644b5b1, every stopping rule left as shipped (maxSeconds = 0, maxReplicates = 96, targetHits = max(10, ntax/5)). Raw cells in dev/profiling/na-certify-stop-hamilton.csv. THE HEADLINE, and it corrects documentation written two commits ago: tripling `targetHits` changed floor attainment on NOT ONE of 30 matrices, while costing 2.58x the wall (26 of 30 slower, 22 by >10%, p = 6e-05). Only 33 of 150 cells were identical in score AND replicate count, so the runs genuinely went longer -- they just never found anything better. The mechanism is the disjoint- population argument measured at corpus scale rather than on one cell: on hard matrices the replicate cap binds before the hit target is reached (hitCapBound rises 22% -> 47%, i.e. the extra demand only pushes more runs into the cap), and on easy matrices the optimum is already in hand. The extra work lands exactly where it cannot help. The `effort` ladder doubles `targetHits` at rungs 5+, so this is evidence against a choice made in b2dfc3a6/2d871b41. It is kept -- without it a notch would be INERT on every dataset that stops early, and under implied weights it deepens the ratchet, which an equal-weights panel cannot see -- but the docs no longer imply it buys reach. Rungs 5+ now read as buying confidence and distinct trees on easy data, and reach on hard data. Gating still costs reach under the real stopping rules, so the default still does not flip: B_gate is 0 better / 4 worse (p = 0.125, uniformly negative), losses concentrated on the hard tail (Zanol2014 -0.6, Wortley2006 -0.4, Aria2015 -0.2, Zhu2013 -0.2). C_final is the arm worth pursuing: statistically indistinguishable from baseline (-0.013, 1 better / 2 worse, p = 1.000) at a THIRD of the wall, and its certifications are the productive ones -- 4349 of 11000 sweeps found a real improver (40%) against A_default's 9555 of 44889 (21%). Certifying the tree a replicate actually reports is four times cheaper and twice as likely to pay. It still regresses on Zanol2014 (0.6 -> 0.0), so it is a candidate, not a change to make now. Settling it wants the hard tail at more seeds, not another corpus-wide sweep: 24 of the 30 matrices sit at 1.0 in every arm and can only dilute the signal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…hypothesis RETRACTION. Panel 2's headline -- "targetHits escalation is pure cost, 0 of 30 matrices improved at 2.6x the wall" -- is true and useless. 24 of the 30 matrices are saturated at 1.0 in every arm, so "no improvement" there is the expected result, not a finding; and floor attainment is binary, so it cannot see an arm getting closer without arriving. I flagged the dilution in the same document and then led with the diluted number anyway. Re-analysed on score, restricted to the six matrices with headroom, and counting what the arm actually bought: extra replicates from targetHits x3, all 30 matrices : 4409 (4.2 CPU-hours) ... on the six matrices with headroom : 243 ... on Zanol2014 / Zhu2013 / Geisler2001 : 0 On the three hardest the 96-replicate cap bound BOTH arms on every seed (21 of 30 headroom cells), so A_hits3 did byte-identical work to A_default -- identical scores, identical replicate counts. 94% of the extra work landed on matrices that were already solved. Only 9 cells anywhere gained a replicate. So the correct claim is not "raising targetHits does not help" but "targetHits cannot act once maxReplicates binds", which is a stronger statement about mechanism and a much weaker one about effort. The hypothesis -- low effort misses the optimum, high effort takes longer and recovers a better score -- was never tested, because the arm could not act where it mattered. PANEL 3 tests it. Hard matrices only (the six with headroom), scored on SCORE not binary attainment, arms = `effort` 0/+1/+2 crossed with certification. Arms are `effort` itself, not a hand-rolled budget, because a first draft that raised only maxReplicates was INERT on Aria2015: targetHits stopped every run at 39 replicates, far below even the 96 cap, so all three budgets returned identical scores. That is the exact mirror of panel 2's mistake. Only moving both knobs escalates every dataset -- which is what the ladder does -- so testing the shipped argument is both more honest and more informative. Two things the smoke test exposed, recorded rather than silently patched: * Panel 2 pinned `strategy = "default"` for every matrix, so on 65-119-tip matrices it did NOT measure what a user gets (`auto` selects `thorough`). * Notch 3->4 is DEAD on hits-bound datasets: rungs 1-4 never touch targetHits and rung 4 raises only the cap, so `effort = +1` and `+2` gave literally the same run on Aria2015. Rung 4 is an auto pick calibrated for >=120 tips, so this is not restructured on no evidence -- panel 3 will quantify it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both fixed together on feature/hsj-token-index-fix. T-374 (HSJ rooting) remains open and unaffected; noted that hsj-paper-oracle.R now reads 8 pass/0 fail (not 2/3) so a future T-374 session doesn't mistake that for a T-374 fix.
…ctors
Tested whether Vine's approach (bioRxiv 10.64898/2025.12.24.696405) -- embed
taxa, decode by neighbour-joining, backpropagate through the decoder -- ports
to TreeSearch. It does not, in either candidate slot.
LeastSquaresTree(): the refit discards the decoder's branch lengths, so
RSS-after-refit is piecewise CONSTANT in the input distance matrix (6 distinct
values over 241 grid points, median adjacent step 0). The only differentiable
surrogate misranks topologies (Spearman +0.10 to +0.36 among RF==2 neighbours),
and scale/affine correction moves that by <0.02, so it is not a branch-length
artifact. An oracle hill-climb on the TRUE objective tied NNI+SPR 3/5 and lost
2/5 -- it never won, so no gradient could help.
Sector starts (build_ras_sector): perturbed-NJ makes better individual starts
than RAS Wagner (2.5 vs 4.0 steps above optimum) but LOSES on best-of-5
(1 win / 7 ties / 7 losses) because its restarts are half as diverse
(CID 0.162 vs 0.328). Multi-start sector solving is diversity-limited, not
start-quality-limited. Corroborated internally: the more diverse pNJ arm beats
the less diverse one on every measure.
Two follow-up leads, unrelated to Vine, recorded in the write-up:
- LeastSquaresTree(method = "ols") returns topologies worse than the untouched
NJ start when rescored under NNLS, on 5/5 targets (20-48% worse). Expected
mathematically, but the documented purpose is Lapointe & Cucumel average
consensus, which is NNLS. Worth a sentence in ?LeastSquaresTree.
- src/ts_ls.{cpp,h} has no certify_unrooted equivalent -- it is the one search
path without the lever that dev/profiling/na-certify-gate.md documents.
Docs and benchmark scripts only; no package code touched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Manual testing underway; shiny app in particular has some usability issues.