Sync cpp-search from ms609 (reverse convergence, post-split) - #182
Merged
Conversation
Update from agent-issues
`effort` doubled the replicate budget and the hit target from rung 5, but not the size of the tree set a search returns -- so a run could be given 8x the budget and still return the default 100 trees, the ceiling bounding the answer rather than the MPT count doing so (observed on four 182-tip analyses, all of which returned exactly `poolMaxSize` trees). Scaling `poolMaxSize` itself would have been wrong. During the replicate loop that cap is not a ceiling on what is returned but the size of the working set the search reads: `tree_fuse` takes its donors from the whole pool (uncapped, `max_rounds = 10`, and held under the pool mutex on the parallel path, where a slow fuse stalls every worker), conflict-guided sectorial search calls compute_split_frequencies() over best-score entries once per replicate, and consensusConstrain calls extract_consensus_splits(). Raising it changes which trees the search VISITS, so the anytime-dominance argument that licenses raising `maxReplicates` -- a higher cap only appends later replicates and never delays an earlier improvement -- does not transfer. After the loop the pool is pure output, and there the argument does hold. So split the knob: new `SearchControl(enumMaxTrees =)` is a ceiling raised only at the MPT-enumeration phase, and `.RungSpec()` gains `enumMultiplier`, doubling from rung 5 in step with the other two budgets. `enumMaxTrees = 0` (the default) keeps `poolMaxSize` throughout, so the shipped default is unchanged; an explicit `enumMaxTrees` outranks the ladder, as an explicit `targetHits` already does. The ceiling is only ever raised, never lowered, so entries already held cannot be orphaned above the cap. It needs no policy cap of its own: enumeration shares the `maxSeconds * enumTimeFraction` reserve and its loop exits as soon as the pool fills, so an over-generous value costs enumeration time, never a worse tree. Applied at all three enumeration sites (both in ts_driven.cpp -- the second inherits the raise, which is unconditional -- and the parallel one, which already operates on the extracted OUTPUT pool, leaving the shared search pool's cap alone). `ts_rcpp.cpp` reads the field defensively so a control list built by an older caller is treated as 0 rather than throwing on R_NilValue. Tests: 919 assertions pass across MaximizeParsimony-features, ts-pool, ts-strategy, ts-driven, ts-parallel, ts_collapse_pool and SearchControl, including an end-to-end case that fails if the field is dropped anywhere along SearchControl -> ctrl list -> DrivenParams -> TreePool::raise_max_size(), and a check that raising the ceiling leaves the score unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat(effort): scale the MPT-enumeration ceiling, not poolMaxSize
Reconcile the two canonical cpp-search lines that diverged after the agent-issues/ms609 repo split: bring agent-issues' 73 newer commits into ms609, leaving both remotes to converge on this merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> # Conflicts: # NEWS.md
Refresh the post-split reconciliation with agent-issues commits landed since this branch was cut (the MaxMin -> Coreset package rename, #180), so the PR is current with its merge target and CI can pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…typo The '-blather' pass shortened the two .PrepareConstraint free-taxa warnings (empty constraint / trivial constraint character), so the free-taxa test's expect_warning() patterns keyed on the old 'constrains nothing' wording no longer matched -- the warnings still fire. Repoint the four patterns at the new wording and fix the 'Igoring' -> 'Ignoring' typo in the empty-constraint message. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sync cpp-search from agent-issues (post-split reconciliation, pre-resolved)
ms609
enabled auto-merge
August 18, 2026 11:47
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
2 participants
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.
Completes the post-split reconciliation begun in ms609 ms609#287: brings ms609
cpp-searchback to agent-issues so both lines hold the same tree.agent-issues
cpp-searchis a strict ancestor of ms609cpp-search, so thisis a conflict-free fast-forward. The 11-commit gap carries:
feat(effort): scale the MPT-enumeration ceiling(+ the Update from agent-issues ms609/TreeSearch#284/feat(effort): scale the MPT-enumeration ceiling, not poolMaxSize ms609/TreeSearch#285 merges);
terseness in
.PrepareConstraint);test-ts-constraint-free-taxa.Rpattern update — these travel together, so thetest stays consistent with the code here (agent-issues currently has the older
"constrains nothing" wording in both).
No new untested code: every commit passed CI on ms609
cpp-search(via ms609#287) orwas already reviewed on agent-issues. Please merge via the UI once the required
checks are green (the "Green to merge" ruleset blocks direct push, by design).
Agent-prepared. Filed under ms609 because the
ms609-agentmachine account iscurrently suspended; the human's account appears as author only for that reason.