Skip to content

fix: add getLastLogProbability() to BasicMapBaumWelchTrainer — parity with BasicBaumWelchTrainer (closes #55)#56

Merged
OldCrow merged 1 commit into
mainfrom
fix/mapbw-last-log-probability
Jul 4, 2026
Merged

fix: add getLastLogProbability() to BasicMapBaumWelchTrainer — parity with BasicBaumWelchTrainer (closes #55)#56
OldCrow merged 1 commit into
mainfrom
fix/mapbw-last-log-probability

Conversation

@OldCrow

@OldCrow OldCrow commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Closes #55.

What changed

BasicMapBaumWelchTrainer::accum_one_sequence already computed logP = fbc.getLogProbability() but discarded it (returned bool). This PR:

  • Changes accum_one_sequence to return double (the sequence log-probability, or -∞ if skipped)
  • Accumulates totalLogProb in train() and stores it as lastLogProb_
  • Exposes getLastLogProbability() const noexcept — identical semantics to BasicBaumWelchTrainer

Semantics: stores the pre-M-step E-step log P(O|λ), reset to -∞ before each train() call.

Tests

3 new tests in test_map_baum_welch.cpp:

  • LastLogProbabilityIsNegInfBeforeTrain — initial state
  • LastLogProbabilityFiniteAfterTrain — finite and negative after one pass
  • LastLogProbabilityMatchesManualSum — equals total_logL on the pre-train model (correct semantics)

47/47 pass.

Co-Authored-By: Oz oz-agent@warp.dev

)

Parity fix with BasicBaumWelchTrainer. Changes:
- accum_one_sequence now returns double (logP) instead of bool
- train() accumulates totalLogProb and stores it as lastLogProb_
- getLastLogProbability() exposes the pre-M-step total E-step log-prob
  (reset to -inf before each train() call)

Three new tests: initial -inf, finite-and-negative after train(),
and pre-train model equality (same semantics as BaumWelchTrainer).

Co-Authored-By: Oz <oz-agent@warp.dev>
@OldCrow OldCrow merged commit 697216f into main Jul 4, 2026
7 checks passed
@OldCrow OldCrow deleted the fix/mapbw-last-log-probability branch July 4, 2026 18:19
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.

fix: BasicMapBaumWelchTrainer missing getLastLogProbability() — parity gap with BasicBaumWelchTrainer

1 participant