Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: CompEvol/MutableAlignment
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: main
Choose a base ref
...
head repository: CompEvol/MutableAlignment
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v2.7
Choose a head ref
Checking mergeability… Don’t worry, you can still create the pull request.
  • 12 commits
  • 3 files changed
  • 2 contributors

Commits on Apr 14, 2026

  1. Fix store/restore safety in MATreeLikelihood and BeagleMATreeLikelihood

    getLogProbsForStateSequence / getLogProbsForPartialsSequence mutate the
    likelihoodCore's tip states and internal partials in place without
    respecting BEAST's store/restore contract:
    
    - calcPatternLogLikelihoods writes partials to whichever slot is "current"
      without first flipping to the scratch slot, so the stored partials
      (captured by store() before the proposal runs) are clobbered and
      restore() cannot recover them.
    - setNodeStates / setTipStates overwrite tip states in place, and there
      is no mechanism to restore the original states on a rejected proposal
      if the mutated tip wasn't in alignment.getDirtySequenceIndices().
    
    This corrupts the "old" state after the first rejected proposal and is
    the likely cause of 100% acceptance rates in operators that use these
    probe methods (e.g. phylonco's ExchangeGibbsOperator).
    
    Fix:
    - Flip the partials buffer to the scratch slot before each probe write,
      tracking flipped nodes so each is flipped at most once per proposal
      (toggling twice would rotate back to the stored slot and clobber it).
    - Track tip nodes transiently mutated via getLogProbs*Sequence, and
      resync them from the (post-restore) alignment on restore() -- a
      superset of alignment.getDirtySequenceIndices() since probes may
      touch tips that weren't alignment-dirty.
    - Clear both tracking sets in store(), accept(), and restore().
    alexeid committed Apr 14, 2026
    Configuration menu
    Copy the full SHA
    0a33bb7 View commit details
    Browse the repository at this point in the history

Commits on Apr 24, 2026

  1. Undo probe-time partials flips before super.calculateLogP()

    Previous fix prevented probe-to-probe poisoning of the stored partials
    slot, but not probe-to-framework poisoning. With hasDirt set to
    Tree.IS_DIRTY by requiresRecalculation(), TreeLikelihood.traverse()
    calls setNodePartialsForUpdate() for every internal ancestor. For
    ancestors previously flipped by getLogProbs*Sequence, this is a second
    toggle in the same proposal: current rotates back to the stored slot,
    calculatePartials writes on top of it, and restore() (an index swap)
    cannot recover.
    
    Fix: in calculateLogP(), undo the flippedNodes toggles before
    super.calculateLogP() runs, so the framework's own traverse-time
    toggles land on the scratch slot instead of the stored slot.
    
    Same change for BeagleMATreeLikelihood using partialBufferHelper.flipOffset().
    
    This is the remaining cause of 100% acceptance in operators that use
    the probe methods (e.g. phylonco's ExchangeGibbsOperator.
    EOF
    )
    alexeid committed Apr 24, 2026
    Configuration menu
    Copy the full SHA
    c99c8f8 View commit details
    Browse the repository at this point in the history

Commits on Apr 25, 2026

  1. Don't clear probe-tracking sets in store()

    The flip-undo machinery added in 0a33bb7/c99c8f8 didn't actually run:
    MCMC calls state.storeCalculationNodes() (and hence MATreeLikelihood.store)
    between operator.proposal() and posterior.calculateLogP() whenever the
    operator requires state initialisation (the default). Probes happen during
    proposal, so clearing tempTipNodes / flippedNodes in store() wiped the
    tracking before calculateLogP() could consume it. Result: probe-time
    buffer flips were never undone, traverse's flip rotated current back to
    the stored slot, and restore() rolled the polluted slot into "current",
    producing the ~30 logP drift observed in phylonco's ExchangeGibbsOperator
    (detected as a likelihood mismatch at sample 0).
    
    Fix: leave the sets populated through the proposal cycle. They are still
    cleared by calculateLogP() (after the undo), accept(), and restore().
    
    Verified: 50K-step short reproducer in beast-phylonco previously failed
    with cached -541.026 vs fresh -570.892 (delta 29.87) at sample 0; now
    passes. 3M-step long test passes cleanly in ~17 minutes.
    alexeid committed Apr 25, 2026
    Configuration menu
    Copy the full SHA
    af09f09 View commit details
    Browse the repository at this point in the history
  2. docs: explain probe APIs and store/restore lifecycle

    Sequence diagrams of the buggy flow (cleared flippedNodes in store) and
    the fixed flow, plus rationale for why a simpler localised design isn't
    adopted (single-buffered tip states + same constraint on BEAGLE path).
    Notes the remaining edge case for operators that probe then return
    Double.NEGATIVE_INFINITY.
    alexeid committed Apr 25, 2026
    Configuration menu
    Copy the full SHA
    bd87476 View commit details
    Browse the repository at this point in the history
  3. docs: define "probe" up front

    The term was used throughout without definition. Added a short opening
    paragraph explaining it means a speculative getLogProbsForStateSequence
    call that side-effects the buffer.
    alexeid committed Apr 25, 2026
    Configuration menu
    Copy the full SHA
    77ff224 View commit details
    Browse the repository at this point in the history
  4. docs: fix mermaid parse error in buggy-flow diagram

    GitHub's mermaid renderer rejected the first sequence diagram, parse
    error around the trailing restore line. Removed the trailing-parens
    "(no-op)" suffix and the c↔s arrow character; substituted plain "and"
    and dropped the inline emoji from the BUG markers. Diagram content is
    the same.
    alexeid committed Apr 25, 2026
    Configuration menu
    Copy the full SHA
    c1f84dd View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    88473cd View commit details
    Browse the repository at this point in the history
  6. Refactor probe partials handling to be hermetic

    Replaces the class-level flippedNodes Set + cross-method undo machinery
    with a localised "flip up, compute, flip back" pattern inside
    calcPatternLogLikelihoods. After the probe call returns, the partials
    indices are exactly what they were on entry, so the framework's
    store/restore can never mishandle them.
    
    Also removes the don't-clear-in-store trap: since there is no longer
    any cross-method probe state for partials, store() can clear normally.
    Only tempTipNodes (for single-buffered tip states) still has to survive
    across the proposal-then-store-then-calculateLogP boundary.
    
    BeagleMATreeLikelihood threads a per-call `flipped` set through the
    rescale recursion so flips are still deduped across rescale attempts
    and flipped back exactly once on success.
    
    Verified: ExchangeGibbsOperatorShortTest still passes.
    alexeid committed Apr 25, 2026
    Configuration menu
    Copy the full SHA
    66e8b99 View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    d8204fa View commit details
    Browse the repository at this point in the history

Commits on Apr 29, 2026

  1. Map alignment column index to tree leaf node by taxon name

    Tree leaf order and alignment column order are independent: they are
    linked only by taxon name, not by integer index. The previous code in
    updateTipData() and the getLogProbs*Sequence probe APIs conflated the
    two, treating tree node number and alignment column index as
    interchangeable. This is correct for synthetic tests where both happen
    to be in the same order (e.g. taxa "0".."4"), but corrupts state when
    the orderings differ (typical for real data, where the alignment is
    written in input/data order while the TaxonSet is alphabetised).
    
    When a probe-based operator (ExchangeGibbsOperator) committed Gibbs-
    sampled sequence to taxon-by-tree-node-number, the alignment wrote it
    to the wrong column. updateTipData() then re-synced tips by taxon
    name, which read the (now stale) right column and ignored the new data
    sitting in the wrong column. Cumulative state corruption across
    pre-burnin tripped the MCMC integrity check.
    
    Build the alignmentIdx <-> treeNodeNr maps once in initAndValidate (by
    name lookup) and use them in updateTipData and the probe APIs.
    dirtySequences and tempTipNodes both now hold alignment column indices
    consistently.
    alexeid committed Apr 29, 2026
    Configuration menu
    Copy the full SHA
    50366a4 View commit details
    Browse the repository at this point in the history
  2. Fix restore() to reuse dirtySequences captured during calculateLogP

    The previous restore() wiped dirtySequences and re-derived it from
    alignment.getDirtySequenceIndices(), but by the time MATreeLikelihood.
    restore() runs the framework has already called alignment.restore(),
    which clears the alignment's edit list. So the re-derivation returned
    nothing for non-probe edits and tip data was never re-synced from the
    rolled-back alignment.
    
    The pre-hermetic-refactor code reused the dirtySequences captured
    during the previous calculateLogP (when the edit list was still
    populated). Restore that pattern. Probe-touched tips (tempTipNodes)
    are still added in case they were not alignment-dirty.
    
    Was masked by ExchangeGibbsOperator passing all edited tips through
    tempTipNodes too, so MCMC scenarios with only that operator looked
    fine. Surfaces the moment any non-probe alignment-edit operator is
    rejected, or any direct setSiteValue test path.
    
    Verified: ExchangeGibbsOperatorShortTest still passes; CRC09 50k chain
    runs to completion without integrity-check failures.
    alexeid committed Apr 29, 2026
    Configuration menu
    Copy the full SHA
    1101943 View commit details
    Browse the repository at this point in the history

Commits on May 13, 2026

  1. Merge pull request #8 from alexeid/fix-store-restore-for-mutable-tips…

    …-beast27
    
    Fix store restore for mutable tips in beast 2.7
    kche309 authored May 13, 2026
    Configuration menu
    Copy the full SHA
    dec8279 View commit details
    Browse the repository at this point in the history
Loading