refactor(concordance)!: chanceCorrect= and unit="nrqs" for QuartetConcordance - #183
Closed
ms609 wants to merge 10 commits into
Closed
refactor(concordance)!: chanceCorrect= and unit="nrqs" for QuartetConcordance#183ms609 wants to merge 10 commits into
ms609 wants to merge 10 commits into
Conversation
…ance
Add `unit = c("quartet", "trit")` to QuartetConcordance(). "quartet"
(default) is unchanged; "trit" scores in Nelson & Ladiges (1992)
redundancy-corrected currency, where a split of sizes (k, t-k) carries
only (k-1)(t-k-1) independent quartets (4-cycles of K_{k,t-k}; the N&L
entailment is GF(2) cycle addition).
Scoring uses coverage ("option b"): per state-pair, concordant trits
A over the reported unit's own content (Wk for edges, Wc for chars),
pooled by shared information M = min(Wc, Wk). Only a split *displayed*
by the character scores 1; nested/crossing get strict partial credit.
Multistate is handled in the same currency, not deferred: a quartet is
decisive only within a state-pair, those K_{n_i,n_j} blocks are
edge-disjoint, and the entailment never crosses them, so trits add over
pairs (W = sum (n_i-1)(n_j-1), weight 4/(n_i n_j)). Verified by GF(2)
rank of the decisive/concordant sets on 2/3/4-state characters. Missing
/ ambiguous / inapplicable tokens drop per character by construction.
Validation (dev/benchmarks/frac-quart/): cell extraction cross-checked
against the C++ kernel; A <= min(Wc, Wk) everywhere; spec ladder
reproduced; package == an independent re-implementation on binary,
multistate and missing-data inputs across all weight x return combos.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Add a random-expectation baseline to QuartetConcordance(unit = "trit"), mirroring ClusteringConcordance()'s normalize interface: normalize = FALSE (default) no correction; published measure unchanged normalize = TRUE exact expected concordance under a fixed-marginal null normalize = <int> Monte-Carlo tip-shuffle estimate of that expectation Null model: reassign each character's tokens across the leaves at random, holding state counts and split sizes fixed (the multivariate-hypergeometric confusion table; the same null as ClusteringConcordance's miRand and Consistency's ExpectedLength). This is the only coherent null for the trit currency -- it re-zeros each (split, char, state-pair) against its own combinatorial floor and extends unchanged to multistate + missing data. The flat 1/3 SCF baseline is a raw-quartet heuristic that does not survive the trit floors; it is documented as the rejected alternative. Implementation: only A (and, for multistate pairs where a pair's side-A count is random, wk and M = min(wc,wk)) vary under the null, so the exact estimator accumulates E[m], E[m*A/wk], E[m*A/wc] from the hypergeometric pmf (cached on (n_i,n_j,M,t)) and re-zeros the pooled edge/char ratio against the pooled expectation, with the same weight aggregation as the observed score. .Rezero guards the z->1 blow-up (-> NA) and returns values unclamped; .Rezero(1,z) = 1 so a displayed split still scores exactly 1 (ceiling invariant preserved), while conflicting characters go negative. Chance correction for the raw unit="quartet" currency is deferred (it needs the per-state-pair cells the C++ kernel does not expose, and a decision on its null) -- normalize errors there for now. Validated: exact vs MC tip-shuffle agree within MC error at the cell level (dev/benchmarks/frac-quart/chance_baseline.R) and end-to-end (test-Concordance.R). Design settled in dev/plans/frac-quart-weight-agreement.md section 7. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…istate
Two gaps in the unit="trit" chance correction, both in the multistate path:
- weight=FALSE re-zero averaged observed and baseline over DIFFERENT
(split, char) cells: observed drops a cell when denM==0 (na.rm), but the
baseline kept it whenever baseDenM>0 (a degenerate multistate pair can have
observed wk=0 yet E[m]>0). Mask both to NA on the union of NA cells so the
edge/char row-means cover the same population before .Rezero. weight=TRUE
(ratio-of-sums) is unaffected.
- The exact-vs-MC end-to-end test used only binary characters, so the
random-wk multistate regime -- the reason E[A] was elaborated into
E[m*A/wk] + E[m] -- had no oracle. Add a 3-state column and cover
edge/char x weight in {TRUE, FALSE}, asserting exact and MC agree on which
cells are NA (guards the cell-matching fix) and within MC error elsewhere.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tet" Extend the chance-correction option to the raw quartet currency, using the same fixed-marginal (hypergeometric) null as the trit path. Per state-pair the raw concordant/decisive counts are polynomials in the 2x2 cells (no floors), so the exact expectation E[conc], E[dec] is a clean sum over the same trivariate-hypergeometric pmf (.ExpectedQuartet / .QuartetExpect); MC (.QuartetMC) reshuffles tokens and re-scores through the C++ kernel. The pooled conc/dec ratio is re-zeroed against E[conc]/E[dec] (weighted) or with the same NA-cell matching as the trit weight=FALSE path (unweighted). Removes the guard that errored on unit="quartet", normalize != FALSE. .Rezero(1, z) = 1, so a displayed split still scores exactly 1; conflicting characters go negative. normalize = FALSE is byte-identical to the published raw measure. Validated exact vs MC tip-shuffle across return x weight x binary/multistate (test-Concordance.R); all Concordance tests green. Design note in dev/plans/frac-quart-weight-agreement.md section 7 updated (raw unit DONE). This is the R reference for the forthcoming C++ port of the same expectation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the exact hypergeometric expectation for QuartetConcordance(normalize = TRUE) from R to C++ (src/concordance_expect.cpp): `quartet_expect` returns E[conc], E[dec]; `trit_expect` returns E[m], E[m*A/wk], E[m*A/wc]. Both sum the per-state-pair trivariate-hypergeometric double sum, hoisting the log-choose terms out of the M/p/r loops (the dominant cost). The R exact helpers (.ExpectedTrit/.CharTritExpect/.ExpectedQuartet and the per-char .QuartetExpect loop) are replaced by thin wrappers / a single all-characters call in .TritConcordance; the Monte-Carlo (normalize = <int>) path stays in R. New Rcpp exports are registered in the manual R_CallMethodDef table (src/TreeSearch-init.c; the package uses R_useDynamicSymbols(FALSE)). Speedup at split-support scale (48 tips, 96 chars): exact normalize=TRUE quartet 2.88s -> 0.14s (~20x), trit 3.48s -> 0.45s (~8x); a full 1000-matrix run drops from ~100 min to ~10 min. Validated bit-for-bit (max |C++ - R| ~1e-14) against a self-contained R reference across binary/multistate/missing data (dev/benchmarks/frac-quart/cpp_expect_parity.R); the exact-vs-MC end-to-end tests in test-Concordance.R now exercise the C++ path and pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`normalize` becomes `chanceCorrect` in `ClusteringConcordance()`, `QuartetConcordance()` and `ConcordanceTable()`. The argument moves the zero point to the value expected under a fixed-marginal null; the scaling to a maximum of 1 was never governed by it, so the old name described something the argument does not do. `unit = "trit"` becomes `unit = "nrqs"`, after the quantity it counts: non-redundant quartet statements. Of the quartets a split of sizes (k, t - k) resolves, (k - 1)(t - k - 1) are non-redundant under the Nelson-Ladiges entailment. `QuartetConcordance()` chance-corrects by default, as `ClusteringConcordance()` already does, so values may be negative where agreement falls below chance expectation. `chanceCorrect = FALSE` gives the uncorrected measure. Documentation gains the caveat that the NRQS ceiling of 1 bounds each split individually, so it holds for `return = "edge"` but not for `return = "char"`, which averages over every split in the tree.
Author
|
Superseded by ms609#276, which carries the same seven concordance commits and predates this branch — I missed it when I opened this, because I searched for The three commits that sat on top here — the Closing in favour of ms609#276. |
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.
Adds a chance-corrected, redundancy-corrected currency to
QuartetConcordance(), and settles the naming of both.Opened under Martin's own account because the
ms609-agentaccount is currently suspended (HTTP 403); this is agent work, not his own authorship.Why now
The simulation study behind "Which characters support which clades?" (accepted, Systematic Biology) calls
QuartetConcordance(unit = "nrqs", chanceCorrect = TRUE)andClusteringConcordance(chanceCorrect = FALSE). Those arguments exist in no commit on any branch of this repository — only in an uncommitted working tree — so the archived study currently names an API that nobody can install. This branch is that working tree.What is here
Seven existing commits add
unit = "trit"and the chance correction; the eighth settles the vocabulary:normalize→chanceCorrectinClusteringConcordance(),QuartetConcordance()andConcordanceTable(). The argument moves the zero point to the fixed-marginal null expectation; it never governed the scaling to a maximum of 1, which is what "normalize" implied.unit = "trit"→unit = "nrqs", after the quantity counted: non-redundant quartet statements. Of the quartets a split of sizes (k, t − k) resolves, (k − 1)(t − k − 1) are non-redundant under the Nelson–Ladiges entailment.QuartetConcordance()chance-corrects by default, asClusteringConcordance()already does. Values may therefore be negative.chanceCorrect = FALSErecovers the measure earlier versions returned.return = "edge", but not forreturn = "char", which averages over every split in the tree.Both renames are breaking.
NEWS.mdcarries the entries.Testing
tests/testthat/test-Concordance.Rpasses in full against a build of this tree (2 visual tests skipped on CRAN). No numeric behaviour changed by the rename commit itself.Before merging
The branch is 7 commits ahead of
cpp-searchbut 371 behind it, so it needs a rebase. I have deliberately not rebased: the study's caches were computed against this exact tree, and I would rather the divergence be resolved deliberately than silently.