Skip to content

fix(mccfr): normalize sampling distribution used for importance reach#57

Merged
krukah merged 1 commit into
mainfrom
k/mccfr-sampling-normalization
Jul 13, 2026
Merged

fix(mccfr): normalize sampling distribution used for importance reach#57
krukah merged 1 commit into
mainfrom
k/mccfr-sampling-normalization

Conversation

@krukah

@krukah krukah commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Fixes #53 — thanks @andyafter for the sharp catch and the function-level pointers.

bug

sampling_distribution() / sampling() weren't normalized. Intended form:

w(a) = max(ε, (σ(a)/τ + β) / (Σσ + β))
Σ w(a) = Z > 1            # smoothing added per-edge in the numerator,
                          # once in the denominator; τ scales only the
                          # numerator; the ε-floor adds more mass

The external sampler draws opponent actions via WeightedIndex, which normalizes internally — so the true probability it drew a is w(a)/Z. But sampling_reach() used the raw w(a) as the importance term in payoff · relative_reach / sampling_reach. Result: counterfactual values mis-scaled by a path-dependent Π Z, compounding as Z^depth.

sizing (live β=2, ε=.05)

infoset shape Z error @ depth 5
early training ≈ K huge (K=5 → ~2800×)
converged + peaked (postflop) ≈ 1.3/node ~5×
shallow / uniform ≈ 1 ~1×

Worst early (β dominates) and persistent postflop (ε-floor on rarely-played sizes); compounds with depth — which is why postflop was the wobble.

fix

Normalize by Z at the single source (sampling_distribution) and route the per-edge sampling() through the same weight, so sampling_reach / ancestor_reach / recursed_value all use the true draw probability.

The draw is unchangedWeightedIndex renormalizes regardless — so this touches only the importance correction, not sampling behavior. Kuhn still converges to the analytical Nash equilibrium.

note

This is a code fix; existing blueprints were trained under the biased correction, so regenerate (--mode reset) to get the corrected strategy — the fingerprint is unchanged, so old tables still load but encode the old fixpoint.

sampling_distribution/sampling() produced unnormalized weights: smoothing
is added per-edge in the numerator but once in the denominator, temperature
scales only the numerator, and the curiosity floor adds mass — so the
per-infoset sum Z is > 1. The external sampler draws via WeightedIndex,
which normalizes internally, so the true draw probability is weight(a)/Z,
but sampling_reach treated the raw weight(a) as the sampling probability.
That mis-scales counterfactual values by a path-dependent product of Z,
compounding worst in deep (postflop) trees.

Normalize by Z at the single source (sampling_distribution) and route the
per-edge sampling() through the same, so sampling_reach / ancestor_reach /
recursed_value all use the true draw probability. The draw itself is
unchanged (WeightedIndex renormalizes regardless), so this corrects only
the importance term; Kuhn still converges to the analytical Nash
equilibrium.

Fixes #53.
@krukah
krukah merged commit 624e4fb into main Jul 13, 2026
6 checks passed
krukah added a commit that referenced this pull request Jul 13, 2026
Asserts sampling_distribution sums to ~1 across Kuhn infosets. Fails on
the pre-#57 code (Z ≈ 1.04 for J|Bet), where the per-edge smoothing
pseudocount and curiosity floor left the distribution unnormalized;
passes with the normalized sampling. Locks in the #53 fix.

Co-authored-by: andyafter <3354145+andyafter@users.noreply.github.com>
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.

MCCFR sampling reach may use unnormalized sampling weights

1 participant