T-200: Decouple R-loop search from MorphyLib; add concavity parameter#216
Closed
ms609 wants to merge 10 commits into
Closed
T-200: Decouple R-loop search from MorphyLib; add concavity parameter#216ms609 wants to merge 10 commits into
ms609 wants to merge 10 commits into
Conversation
- New R/NativeSearch.R: PrepareNativeData, CleanNativeData, NativeLength, NativeBootstrap (4 exported functions) - TreeSearch(), Ratchet(), Jackknife() now default to native scorer - New 'concavity' parameter on all three functions - Jackknife() rewritten to support both native and morphy paths - EdgeListSearch() default scorer changed to NativeLength - Deprecated: PhyDat2Morphy, UnloadMorphy, MorphyLength, MorphyTreeLength, MorphyBootstrap - Vignette custom.Rmd IW section rewritten (was ~150 lines of MorphyLib scaffolding, now 30 lines using concavity parameter) - test-NativeSearch.R: 7 tests covering all new functions - test-CustomSearch.R: updated to use native defaults
Adds NativeSearch.Rd and updates Jackknife.Rd, Ratchet.Rd, TreeSearch.Rd with new concavity parameter and default changes.
ms609
force-pushed
the
feature/native-search
branch
from
e25cbab to
f59a193
Compare
- inst/WORDLIST: add 'cleanup' and 'phyDat' - R/mpl_morphy_objects.R: wrap PhyDat2Morphy/MorphyWeights @examples in \donttest{}; use suppressWarnings() for UnloadMorphy in GapHandler and SingleCharMorphy examples - R/Morphy.R: wrap constraint example with suppressWarnings() so internal PhyDat2Morphy/UnloadMorphy calls don't emit warnings Fixes 1 ERROR (spelling) + 1 WARNING (deprecated fns in examples)
Wrap PhyDat2Morphy() and UnloadMorphy() calls in suppressWarnings()
in the \examples{} sections of GapHandler.Rd, MorphyWeights.Rd,
PhyDat2Morphy.Rd, RearrangeEdges.Rd, and SingleCharMorphy.Rd.
Wrap Morphy() in suppressWarnings() in the \donttest{} block of
Morphy.Rd -- Morphy() calls PhyDat2Morphy/UnloadMorphy internally via
lapply/vapply, generating the 'FUN' is deprecated warning stream.
R CMD check --as-cran treats warnings-in-examples as errors.
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>
Owner
Author
|
Superseded in #251 |
ms609
added a commit
that referenced
this pull request
Jul 6, 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>
ms609
added a commit
that referenced
this pull request
Jul 9, 2026
* Score MorphyLength via native kernel for inapplicable data (Stage 1 of 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>
* Document staged plan for dropping MorphyLib entirely (Stages 2-3)
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>
* Remove Morphy() entirely (Stage 2 of dropping MorphyLib)
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>
* Remove vendored MorphyLib; native functional resampling
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>
* Native scoring layer: purpose-based names + concavity (best of #216)
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>
* DESCRIPTION: drop MorphyLib copyright; recast BGS contributors as ctb
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>
* DESCRIPTION: add Guillerme ORCID
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Regenerate docs after merging cpp-search
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).
* Deprecate InitializeData/CleanUpData; move concavity onto PrepareData()
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.
* Fix custom.Rmd: IWScore's minLength was recomputed on every call
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.
* Revise IW examples
* Rewrite
* Missing @imports
* importFrom TreeTools AddUnconstrained
* Split out Morphy.R
& rm dud "deprecated" new MaxPars2
* MaximizeParsimony2 removal
* Delete Sectorial.R
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
* Update Collate per deleted files
* Rm obselete tests
* Delete run_tests.R
* Spellings
* -@
* syntax error @Vinh2010
* Document Resample()'s live args; deprecate its dead ones
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>
* Fix R-CMD-check 6h timeout: gate WideSample()'s solver-tier demo behind 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.
* Document Resample() via @inheritParams MaximizeParsimony; fix nThreads=0
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>
* Support inapplicable = "missing" in Resample()
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>
* rm cruft
* Redoc Jackknife
* Redoc jackknife
* Update blurb
* -Morphy
* Quiet tests
* Message suppression
* rm unneeded comment
* No morphy → no Postorder needed
* -morphy refs
* Quieten
* Rm Morphy refs
* Rm stale Morphy references
* Faster example
(!)
* Resample dox
* -stale RoxygenNote
* dup param
* Delete bench-iw.R
Obselete
* Update MorphyLib migration doc: mark plan complete, flag divergences
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.
* Delete morphy-migration.md
* Compact
* obselete
* Delete 2026-06-15-morphylib-removal-stages.md
* Rm redundant / obsolete
* Update test-ts-na-ambig.R
* -Simd refs; rm empty test
* Format
* Rm morphy refs
* clarity
* Redoc
* Delete debug_ratchet.R
rm stray debug file
* Standalone `Resample()`
* Restore example setup
* Update Resample.Rd
---------
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.
Agent B
Summary
Replaces MorphyLib scoring in
TreeSearch(),Ratchet(),Jackknife()withnative C++ engine (
ts_fitch_score()), while retaining backward compatibilityfor users passing MorphyLib scorers explicitly.
New exports
PrepareNativeData()— replacesPhyDat2Morphy()CleanNativeData()— replacesUnloadMorphy()NativeLength()— replacesMorphyLength()NativeBootstrap()— replacesMorphyBootstrap()Changed defaults
TreeSearch,Ratchet,Jackknife, andEdgeListSearchnow default tonative scorers. New
concavityparameter (defaultInf= equal weights)makes implied-weights searches trivial:
Deprecations
PhyDat2Morphy,UnloadMorphy,MorphyLength,MorphyTreeLength,MorphyBootstrapemit.Deprecated()warnings.Vignette
custom.RmdIW section rewritten from ~150 lines of MorphyLib scaffoldingto ~30 lines.
Tests
7 new tests in
test-NativeSearch.R. Existing tests updated to use nativedefaults. 50 pass / 0 fail.
Note
Native scorer uses simple IW (
fit = e / (e + k)), matchingTreeLength(..., extended_iw = FALSE). XPIWE (Extension 3) remains in theC++ search path.