Profiling target, not a correctness problem — sampling is right either way.
random_constrained_by_insertion() (src/ts_wagner.cpp, added by #122) runs a
rejection pass before its filtered one:
for (attempt = 0; attempt < 64 && !uniform; ++attempt) {
shuffle the named tips
build an unconstrained random tree on them
remap_partial() // full update_constraint()
uniform = every split displayed
}
It buys exact uniformity when it lands. When it cannot land it is pure overhead:
all 64 attempts are built, remapped, and thrown away, and the filtered pass then
runs from scratch. And "cannot land" is not a rare corner — the acceptance rate
falls off fast with the size of the constrained groups. For a single character
with a "together" and b "apart" taxa it is
T(a+1) · T(b+1) / T(a+b), T(k) = (2k-5)!!
which is ~2% at 4-vs-4 (fine, lands in ~46 tries) and ~1e-7 at 10-vs-10
(hopeless). So a user who constrains a moderately sized clade pays the full 64
attempts on every replicate and gets nothing for them.
What is known
- 60 taxa, 40 named 20/20, 20 free — a configuration where rejection never lands
— draws at 6.2 ms. That is the wasted-rejection case, but the measurement
does not separate the 64 discarded attempts from the filtered pass that
follows, so the first job is to split it.
- The backbone path (constraints naming every taxon) does not go through any of
this and is much cheaper.
- Blast radius is limited: this is the
RANDOM_TREE start strategy only, one arm
of the adaptive start mix, once per replicate. It is not on the TBR hot path.
Leads, cheapest first
- Predict instead of trying. The acceptance rate above is computable from
the group sizes before the first attempt. Skip pass 1 entirely when the
estimate says it cannot land within the budget — this removes the whole cost
in exactly the case where it is wasted, and changes nothing where it is not.
Multiple characters make the formula an upper bound rather than exact, which
is the right direction for a skip test.
- Make an attempt cheaper. Each one ends in a full
update_constraint() —
compute_node_tips() over every node plus a postorder scan per split — only
to answer a yes/no. A cheaper compliance test (or an incremental one that
rejects as soon as a split is broken, mid-build, rather than at the end) would
cut the per-attempt cost several-fold.
- 64 is arbitrary. It was picked to be obviously affordable, not measured.
With (1) in place the right number may well be different, and may want to
scale with tip count.
- Failing all that, cache nothing and just bail after the first few attempts
when the constraint is large — a cruder version of (1).
Not in scope
Replacing the filtered fallback itself. It reaches every compliant topology
(verified exhaustively in tests/testthat/test-ts-random-constrained-free.R);
it is only its uniformity that pass 1 exists to improve, and that trade is
already documented in the code, the vignette and NEWS.
Reopening condition if closed without a fix: a profiling run showing constrained
RANDOM_TREE starts as a non-trivial share of replicate wall time.
See dev/profiling/ and the /profile standing practice.
Profiling target, not a correctness problem — sampling is right either way.
random_constrained_by_insertion()(src/ts_wagner.cpp, added by #122) runs arejection pass before its filtered one:
It buys exact uniformity when it lands. When it cannot land it is pure overhead:
all 64 attempts are built, remapped, and thrown away, and the filtered pass then
runs from scratch. And "cannot land" is not a rare corner — the acceptance rate
falls off fast with the size of the constrained groups. For a single character
with
a"together" andb"apart" taxa it iswhich is ~2% at 4-vs-4 (fine, lands in ~46 tries) and ~1e-7 at 10-vs-10
(hopeless). So a user who constrains a moderately sized clade pays the full 64
attempts on every replicate and gets nothing for them.
What is known
— draws at 6.2 ms. That is the wasted-rejection case, but the measurement
does not separate the 64 discarded attempts from the filtered pass that
follows, so the first job is to split it.
this and is much cheaper.
RANDOM_TREEstart strategy only, one armof the adaptive start mix, once per replicate. It is not on the TBR hot path.
Leads, cheapest first
the group sizes before the first attempt. Skip pass 1 entirely when the
estimate says it cannot land within the budget — this removes the whole cost
in exactly the case where it is wasted, and changes nothing where it is not.
Multiple characters make the formula an upper bound rather than exact, which
is the right direction for a skip test.
update_constraint()—compute_node_tips()over every node plus a postorder scan per split — onlyto answer a yes/no. A cheaper compliance test (or an incremental one that
rejects as soon as a split is broken, mid-build, rather than at the end) would
cut the per-attempt cost several-fold.
With (1) in place the right number may well be different, and may want to
scale with tip count.
when the constraint is large — a cruder version of (1).
Not in scope
Replacing the filtered fallback itself. It reaches every compliant topology
(verified exhaustively in
tests/testthat/test-ts-random-constrained-free.R);it is only its uniformity that pass 1 exists to improve, and that trade is
already documented in the code, the vignette and NEWS.
Reopening condition if closed without a fix: a profiling run showing constrained
RANDOM_TREEstarts as a non-trivial share of replicate wall time.See
dev/profiling/and the/profilestanding practice.