Replace Morphy with native kernel for inapplicable data#251
Merged
Conversation
ms609
added a commit
that referenced
this pull request
Jun 16, 2026
Synthesise the best of PR #251 (this branch) and PR #216 (T-200, Agent B), which independently decoupled the R-loop search from MorphyLib. Adopted from #216: - Efficient native scorer: PrepareData() pre-extracts the scoring matrices once and EdgeListScore() calls ts_fitch_score() directly, instead of rebuilding a phylo and re-extracting tip data on every score. - `concavity` parameter on TreeSearch()/Ratchet()/Jackknife(): Inf = equal weights (default), finite = implied weights, "profile" = profile parsimony. Custom InitializeData hooks still receive the dataset unchanged. - custom.Rmd implied-weights section rewritten from ~150 lines of hand-rolled scaffolding to a few lines using `concavity` directly. Kept from #251 (and fixing #216's staleness): - MorphyLib stays fully deleted (#216 retained it as a fallback). - Score weights use .ScaleWeight() (fractional-weight aware) and Inf for the equal-weights concavity, matching current TreeLength(); #216 used as.integer(weight) and -1.0, predating those changes. - Functional resampling + a regression test (doubling weights doubles score). Naming: the layer is renamed to purpose-based names rather than #216's `*Native*` prefix, because once MorphyLib is gone "Native" no longer contrasts with anything. PhyDat2Morphy->PrepareData, UnloadMorphy->ReleaseData, is.morphyPtr->is.ParsimonyData, SingleCharMorphy->SingleCharData, MorphyLength->EdgeListScore, MorphyTreeLength->TreeScore, MorphyBootstrap->BootstrapTree, RandomMorphyTree->RandomPostorderTree. The old names remain as .Deprecated() aliases. TreeScore() == TreeLength() verified across equal weights, implied weights and profile parsimony; full suite green (only the pre-existing WideSample/MaxMin API-drift failures remain, unrelated to this change). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…f MorphyLib removal)
MorphyLength()/MorphyTreeLength() route the default inapplicable gap mode
through the native Fitch kernel (FastCharacterLength) instead of vendored
MorphyLib. Fixes a known MorphyLib bug scoring `{1-}` ambiguous-with-
inapplicable tokens (Lobo pattern 93: native 2 vs MorphyLib 3), aligning the
legacy custom-search scorers (TreeSearch, Ratchet, Jackknife, custom.Rmd)
with the already-correct MaximizeParsimony.
- PhyDat2Morphy()/SingleCharMorphy() attach precomputed native scoring data
(.NativeMorphyData/.SingleCharNative); the Morphy pointer is retained so
Morphy()'s C-loop and introspection are untouched.
- Rare ambiguous/extra gap modes keep the MorphyLib path for now.
- Not aiming for MorphyLib-equivalence: native scores are the correct v2.0
behaviour. Verified MorphyTreeLength == TreeLength across Lobo + CL
datasets and per character via SingleCharMorphy.
Stage 1 of dropping MorphyLib entirely from 2.0; later stages migrate
Morphy()'s preorder_morphy/morphy_iw/morphy_profile loop and delete the
vendored sources.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Captures the migration of Morphy()'s preorder_morphy/morphy_iw/morphy_profile loop and the ambiguous/extra gap modes to native (Stage 2), and the deletion of vendored MorphyLib sources/bindings/introspection (Stage 3). Flags the Morphy() approach (swap closures vs redirect to MaximizeParsimony) as needing maintainer review, since both change superseded-path behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Morphy() was the redundant MorphyLib R-loop search (EW/IW/profile + ratchet + constraints), never released on CRAN (not in 1.8.0), sitting between native MaximizeParsimony() and the custom-criteria TreeSearch()/Ratchet(). Everything it did is covered natively, so it is deleted rather than migrated. - Delete Morphy() + its private helpers (.EWScore/.IWScore/.ProfileScore/ .TBRSearch/.CombineResults/.ReplaceResults/.Time/.DateTime) from R/Morphy.R. - Replace MaximizeParsimony()'s legacy-parameter delegation (do.call(Morphy,..)) with an informative error. - Drop export(Morphy); delete man/Morphy.Rd + man/dot-CombineResults.Rd; fix dangling [Morphy()] doc links (WhenFirstHit, EasyTrees, MaximizeParsimony). - Delete test-Morphy.R (+ snapshots); drop Morphy() calls from test-CustomSearch. Leaves morphy_iw/morphy_profile dead (Morphy() was their only caller), to be removed in Stage 3. Full suite green: FAIL 0, PASS 11201. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the vendored Morphy Phylogenetic Library entirely. All parsimony
scoring now runs through the native C++ kernel, which implements the
Brazeau-Guillerme-Smith inapplicable algorithm correctly (including the
`{1-}` token MorphyLib mis-scored). The native engine's own Wagner
builder (ts_wagner.cpp) is independent of MorphyLib, so the library
deletes wholesale - no MPLstate extraction needed.
Custom-search resampling (Jackknife/MorphyBootstrap) now resamples
character weights functionally and scores them with the native kernel,
replacing the in-place mpl_set_charac_weight mutation that the Stage-1
native MorphyLength path silently ignored (a latent regression). The
custom TreeScorer/EdgeSwapper API is unchanged; a new regression test
asserts resampled weights affect the score.
- PhyDat2Morphy/SingleCharMorphy native-only; UnloadMorphy a no-op;
RandomTreeScore native (kept RandomMorphyTree + pure C RANDOM_TREE)
- Removed mpl_* bindings, MorphyWeights/SetMorphyWeights/GapHandler/
MorphyErrorCheck/summary.morphyPtr, GetMorphyLength/C_MorphyLength,
MORPHYLENGTH/RANDOM_TREE_SCORE, morphy_iw/morphy_profile/preorder_morphy
- Deleted 17 vendored MorphyLib C/C++ sources; trimmed TreeSearch-init.c
and build_postorder.h; regenerated RcppExports/NAMESPACE/man
Only the default `inapplicable` gap treatment is supported natively.
Full suite green; the only failures are pre-existing WideSample/MaxMin
API drift, unrelated to this change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Synthesise the best of PR #251 (this branch) and PR #216 (T-200, Agent B), which independently decoupled the R-loop search from MorphyLib. Adopted from #216: - Efficient native scorer: PrepareData() pre-extracts the scoring matrices once and EdgeListScore() calls ts_fitch_score() directly, instead of rebuilding a phylo and re-extracting tip data on every score. - `concavity` parameter on TreeSearch()/Ratchet()/Jackknife(): Inf = equal weights (default), finite = implied weights, "profile" = profile parsimony. Custom InitializeData hooks still receive the dataset unchanged. - custom.Rmd implied-weights section rewritten from ~150 lines of hand-rolled scaffolding to a few lines using `concavity` directly. Kept from #251 (and fixing #216's staleness): - MorphyLib stays fully deleted (#216 retained it as a fallback). - Score weights use .ScaleWeight() (fractional-weight aware) and Inf for the equal-weights concavity, matching current TreeLength(); #216 used as.integer(weight) and -1.0, predating those changes. - Functional resampling + a regression test (doubling weights doubles score). Naming: the layer is renamed to purpose-based names rather than #216's `*Native*` prefix, because once MorphyLib is gone "Native" no longer contrasts with anything. PhyDat2Morphy->PrepareData, UnloadMorphy->ReleaseData, is.morphyPtr->is.ParsimonyData, SingleCharMorphy->SingleCharData, MorphyLength->EdgeListScore, MorphyTreeLength->TreeScore, MorphyBootstrap->BootstrapTree, RandomMorphyTree->RandomPostorderTree. The old names remain as .Deprecated() aliases. TreeScore() == TreeLength() verified across equal weights, implied weights and profile parsimony; full suite green (only the pre-existing WideSample/MaxMin API-drift failures remain, unrelated to this change). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MorphyLib C/C++ code was fully removed in 33dc949; the Copyright field is now factually wrong. Drop it. Brazeau and Guillerme contributed the BGS inapplicable-data algorithm, not a vendored library, so both are recredited as ctb with an accurate comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sync roxygen cross-links following the rebase onto current cpp-search (picks up upstream's BigCamelCase hierarchy-helper renames alongside this branch's own renames).
ms609
force-pushed
the
feature/native-morphy-helpers
branch
from
dc50017 to
d324545
Compare
With MorphyLib gone, neither hook does anything a user can't do just as well themselves: InitializeData/CleanUpData were called exactly once, single-threaded, to build/free a C pointer that no longer exists (native ParsimonyData is plain, GC'd R data). TreeSearch()/Ratchet()/Jackknife() now pass `dataset` straight through to TreeScorer; callers prepare it themselves (typically via PrepareData()) before calling. Supplying the old hooks still works but warns via .Deprecated(). concavity moves off these three functions onto PrepareData(), since it belongs to the dataset, not the search call -- the default TreeScorer, EdgeListScore(), reads it from the prepared object directly. Confirmed empirically that BootstrapTree()'s reweighting-only resampling already handles IW correctly, including under mostly-zero pattern weights. Rewrites the custom.Rmd implied-weights section, which no longer needed any of the removed machinery, to instead show how to hand-write a scorer of comparable complexity (MinimumLength()/CharacterLength()-based, with concavity captured by closure) as a template for optimality criteria TreeSearch doesn't provide natively.
minLength = MinimumLength(dataset, compress = TRUE) as a default argument is a lazy promise evaluated fresh each call, since nothing threads minLength= through the search loop -- defeating the "compute once, cache" example it was meant to demonstrate. Drop it as a formal parameter; the function body now closes over the pre-computed outer `minLength` instead (a cheap lookup, not a recomputation). Verified the closure is actually reused across all evaluations in a search, and that scores still match the native implementation exactly.
& rm dud "deprecated" new MaxPars2
Doesn't benefit from computational time-saving without a custom-written scorer. Unlikely to escape from optima any better than Ratchet() Hence unclear what niche this would fill. Verdict = retire
Resample.Rd's undocumented-argument NOTE traced to two separate causes. Resample() used to share Morphy()'s roxygen block via `@rdname Morphy`, which was deleted along with Morphy() itself (d324ae1); that accounts for dataset/tree/ratchIter/tbrIter/concavity/constraint, which are all still live and just needed honest new docs describing what they map to in the native search engine (ratchIter/tbrIter in particular have been silently repurposed from their old Morphy()-era meaning). Separately, finalIter/maxHits/tolerance/verbosity have done nothing since Resample() was first rewritten around the native engine -- they were never wired into ts_resample_search/ts_parallel_resample, which have no equivalent controls (no progress-reporting hook exists at all). Rather than write misleading docs for dead parameters on a soon-to-be-frozen API, deprecate them (default NULL; a warning fires only if explicitly supplied), matching the InitializeData/CleanUpData deprecation already applied elsewhere in this PR. Drop the corresponding now-inert `verbosity = 0` from the tree-search.Rmd example. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nd interactive() The R 4.1 and R-devel legs were hanging for the full 6h GHA limit at "checking examples with --run-donttest", while release (sense-check, windows) passed in minutes. Comparing job logs pinned the difference to how the 'highs' package builds: on those two legs it compiles its bundled HiGHS solver from source via cmake (no RSPM binary exists for an archived old version or for R-devel), whereas the passing legs use a prebuilt binary. WideSample.Rd's donttest example exercised the highs-backed exact solver (both explicitly via effort = 4 and implicitly via effort = NULL's auto tier-selection), and a from-source HiGHS build is apparently slow enough to blow through the documented 60s maxSeconds budget by hours. Since MaxMin/highs are Suggests-only and the DropAdd/Grasp/exact tiers' runtime is inherently environment-dependent (and, per direct testing against both the CI-pinned and current MaxMin HEAD, its calling convention is also actively in flux upstream), move every example beyond the deterministic, matrix-free FarFirst tier behind `if (interactive())`, matching the existing convention for expensive optional demos (e.g. Morphy.Rd). Only the always-safe FarFirst call now runs under automated checks.
Resample() is a MaximizeParsimony() relative, not a TreeSearch()-family function, so its shared params (dataset, concavity, constraint, hierarchy, inapplicable, hsj_alpha, extended_iw, xpiwe_r, xpiwe_max_f) should inherit MaximizeParsimony()'s canonical descriptions instead of restating them, keeping the two docs from drifting apart. Params where Resample()'s behaviour genuinely differs (tree: fallback not warm-start; the deprecated finalIter/maxHits/tolerance/verbosity quartet) keep explicit overrides, which roxygen2's @inheritParams respects. While aligning nThreads's documentation with MaximizeParsimony(), found Resample() silently clamped nThreads to a minimum of 1, defeating the 0 = auto-detect behaviour that MaximizeParsimony() documents and that the underlying ts::parallel_resample() engine already implements. Fixed to pass nThreads through unclamped, matching MaximizeParsimony(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resample()'s match.arg only accepted "bgs"/"hsj"/"xform", so the pure-Fitch mode already demonstrated in Resample()'s own @examples (inapp = "miss") errored. Recode gaps as missing via .GapsAsMissing() and fall through the bgs path, matching MaximizeParsimony()'s treatment exactly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Obselete
Trims completed tables/phases now that Tiers 0-4 and Phases A/C/D are all done on this branch, and records where the actual outcome diverged from what was planned: Morphy() was deleted rather than migrated, the rare gap treatments were dropped rather than reproduced, and InitializeData/CleanUpData were deprecated rather than merely re-pointed at native defaults. Notes the one thing still open: merging #251 into cpp-search, which still carries the full vendored library.
ms609
changed the title
…y.R conflict Conflict was purely additive: this branch made no changes in the affected region (only doc-only MorphyLib-removal work elsewhere), so resolution takes cpp-search's rebase of the `large` strategy preset onto `thorough` plus the `intensive` alias.
ms609
added a commit
that referenced
this pull request
Jul 9, 2026
Audit of the documentation surface touched by the 2.0.0 release. Stale/incorrect claims corrected: - Preset docs described the retired bespoke `large` preset (biased Wagner, annealing, reduced cycles); `large` is now an alias of `thorough` with maxReplicates=500. Fixed in MaximizeParsimony roxygen, the SearchControl comment, and the search-algorithm / tree-search vignettes. - `default` preset documented as 12 ratchet cycles; it is 6. - Vignette preset claims about adaptiveStart, ratchetTaper, annealCycles and consensusStableReps contradicted the actual SearchControl defaults / preset values. - MaximizeParsimony profile-parsimony docstring claimed binary-only (max 2 states); multi-state is supported (Carter 1990 / Maddison & Slatkin 1991 / Monte Carlo). ConcordantInformation carried the same stale two-state restriction. - PrepareDataProfile: `bootstrap` attribute documented a `split.sizes` entry it never writes; "with a warning" overstated (only the inapplicable-token simplification emits a message). - WideSample `@return` said "uniform sample"; it returns a Max-Min diverse subset. - fractional-weight default documented as 1000L; it is 1260L (NEWS.md). - Implied-weight fit formula rendered as `k / e + k`; corrected to `k / (e + k)` (propagates to inheriting man pages). LLM artefacts removed: - The "reflects island structure, not a cap" framing in the Completeness section (the "256 is not hard-coded" aside was already removed on the branch tip by #251). - NEWS.md: the "common complaint" defensive aside, the empty "Other improvements" section, and a duplicate PaintCharacters bullet. man/*.Rd regenerated with roxygen2. Not addressed here (left for the maintainer): the "To integrate into 2.0.0 notes" scaffold in NEWS.md now also holds #251's breaking-change bullets, and the "renamed to Morphy()" line is stale since #251 removed Morphy() — final integration of that section is #251's to make. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
ms609
added a commit
that referenced
this pull request
Jul 10, 2026
Brings the Bremer branch up to current cpp-search, notably the native inapplicable kernel replacing Morphy (#251) and the FixedTree change. Conflict resolutions: - R/MaximizeParsimony.R: keep the scoring-signature helpers (.ScoringSignature / .ScoringSignatureMatch / .DescribeScoring) added for Bremer provenance; honour cpp-search's removal of the deprecated MaximizeParsimony2 alias and its EasyTrees launcher. - DESCRIPTION Collate / NAMESPACE / man: regenerated with roxygen; drop Sectorial.R (deleted upstream) and C_MorphyLength (Morphy removed), keep SuboptimalTrees + Bremer exports. - RcppExports regenerated: Morphy exports dropped, negative-constraint plumbing retained. Also files SuboptimalTrees() under @family tree scoring (in addition to split support functions). Bremer, SuboptimalTrees and constraint suites: 126 pass, 0 fail. Full recompile of the merged C++ is clean. Co-Authored-By: Claude Opus 4.8 <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.
What & why
Routes
MorphyLength()/MorphyTreeLength()for the default inapplicable gap mode through the native Fitch kernel (FastCharacterLength) instead of vendored MorphyLib.This fixes a known MorphyLib bug scoring
{1-}(ambiguous-with-inapplicable) tokens — e.g. Lobo pattern 93 scores 2 (correct BGS) vs MorphyLib 3. Legacy custom-search scorers (TreeSearch,Ratchet,Jackknife, the custom-criteria vignette) now match the already-correctMaximizeParsimony(native kernel).Per maintainer decision: not aiming for MorphyLib bit-equivalence — native scores are the correct v2.0 behaviour.
How
PhyDat2Morphy()/SingleCharMorphy()attach precomputed native scoring data (.NativeMorphyData/.SingleCharNative). The Morphy pointer is retained, soMorphy()s C-loop and MorphyLib introspection are untouched.ambiguous/extragap modes keep the MorphyLib path for now (helper returnsNULL-> falls through).Validation
MorphyTreeLength == round(TreeLength)and per-characterSingleCharMorphysums ==TreeLength, verified across Lobo + congreveLamsdell datasets on pectinate & balanced trees (newtest-native-morphy-equivalence.R). Lobo pectinate = 273 (was 274 under the MorphyLib bug). NB CI may surface legacy tests that assert the old MorphyLib scores; those should be updated to the native-correct values.Staging (drop MorphyLib entirely from 2.0)
MorphyLengthpath.Morphy()spreorder_morphy/morphy_iw/morphy_profileloop + ambiguous/extra modes to native.mpl_*bindings, introspection helpers; update DESCRIPTION/Makevars.🤖 Generated with Claude Code