Skip to content

getNumberOfTrees: report the support, not the graph - #17

Open
alexeid wants to merge 1 commit into
masterfrom
fix/number-of-trees-contract
Open

getNumberOfTrees: report the support, not the graph#17
alexeid wants to merge 1 commit into
masterfrom
fix/number-of-trees-contract

Conversation

@alexeid

@alexeid alexeid commented Aug 11, 2026

Copy link
Copy Markdown
Member

ITreeDistribution.getNumberOfTrees() was documented only as "the number of trees (topologies) in this distribution", which is ambiguous between the CCD graph and the distribution's support. They coincide for CCD0/CCD1 but not for full-support models.

KRegCCD and MRegCCD both override containsTree to return true, yet inherited the graph-based count from AbstractCCD. On a 129-taxon posterior KRegCCD reported log 88 against a true support of log 582, contradicting its own containsTree.

  • document the contract on the interface: support, not graph
  • override in KRegCCD and MRegCCD
  • move numberOfRootedTopologies and logBigInteger to AbstractCCD so all models share them; UniformEscapeCCD.numberOfRootedTopologies still delegates
  • delete a commented-out logBigInteger in CredibleCCDComputer that was incorrect (new BigDecimal(bigInteger).scale() is always 0, so it reduced to Math.log(val.doubleValue()))
  • add NumberOfTreesContractTest

Note the double-overflow boundary is ~155 taxa: 129 taxa needs 840 bits and converts fine, 160 needs 1093 and overflows. Coal160/Coal320/Yule200/Yule400 are past it.

Existing callers (NNICladeExpansion, RogueDetection) use CCD0/CCD1 where graph and support coincide, so are unaffected. Full suite: 110 tests, 0 failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant