Skip to content

examples: fix the enlarge driver's unreachable closure test - #40

Merged
Sophia Wen (hfwen0502) merged 1 commit into
mainfrom
fix-enlarge-expansion-bugs
Sep 21, 2026
Merged

Sophia Wen (hfwen0502) merged 1 commit into
mainfrom
fix-enlarge-expansion-bugs

Conversation

@hfwen0502

@hfwen0502 Sophia Wen (hfwen0502) commented Sep 19, 2026

Copy link
Copy Markdown
Member

Summary

enlarge_via_singles() builds its bitstring rows from the threshold-selected
strings only:

ia_idx, ib_idx = np.nonzero(weights > threshold)
alpha_bits = ints_to_bool_columns(np.asarray(ci_strs_a)[ia_idx], norb)

Row 0 of transitions is the identity, so everything fed in comes back — but
a solved string whose every pair fell below --enlarge_threshold is never fed
in, and never comes back. So the result is a superset of the dominant
strings, not of the subspace. The docstring claimed the opposite ("Always a
superset of the input ci_strs_a/ci_strs_b"), and the loop's closure test
believed it and tested equality.

Equality additionally requires every solved string to be dominant, which is
false on any subspace with a low-weight tail. So no_growth never became
true, the "expansion added nothing new" stop reason was dead code, and every
run paid an extra round to establish energy/occupancy convergence — negligible
on a small molecule, minutes on anything large.

Fix: the subset test the condition always meant, matching
run_sqd_sbd_carryover.py:

no_growth = (set(new_alpha.tolist()) <= set(int(x) for x in ci_strs_a)
             and set(new_beta.tolist()) <= set(int(x) for x in ci_strs_b))

Still computed on the raw expansion, before cap_to_max_dim — capping
first would mask real growth and could stall a run while reporting
convergence. Docstring corrected to describe the pruning. A nearby --max_dim
comment is also made dataset-agnostic.

Verification (h100, 8 GPU ranks, H2O)

main and the fix back to back, same flags:

rounds stop reason energy final subspace
main 3 energy converged -76.2421767512 1742 x 1742
this PR 2 expansion added nothing new -76.2421767512 1742 x 1742

Identical energy to ten digits over an identical final subspace — the fix
changes when closure is recognised, not what the loop converges to.

Round 2 is the bug in miniature: the expansion is 1720 strings against
1742 solved. A strict subset adding nothing — but 1720 != 1742, so
equality declared growth. (Same two numbers #39's description explains as a
quirk of this test; this is the cause.)

Notes for review

  • Scoped to this defect. The module docstring still references a
    run_sbd_selected_ci.py that no longer exists; examples: SQD driver that grows its subspace via SBD's own carryover #39 already corrects those,
    so fixing them here would only conflict.
  • cap_to_max_dim is untouched. Separately, once the include list reaches
    max_dim it fills upstream's whole budget on its own, since upstream builds
    unique(concat(include_a, include_b, carryover, samples))[:max_dim] — a
    design question, not part of this fix.

enlarge_via_singles() builds its bitstrings from the threshold-selected
strings only, so it returns a superset of the *dominant* input strings, not
of the subspace: a solved string whose every pair fell below
--enlarge_threshold is never fed in and never comes back. The docstring
claimed the opposite, and the loop's closure test believed it and used
equality -- which additionally requires every solved string to be dominant.
On any subspace with a low-weight tail that never held, so no_growth never
fired, "expansion added nothing new" was dead code, and every run paid an
extra round to establish energy/occupancy convergence.

Replaced with the subset test the condition always meant (the same one
run_sqd_sbd_carryover.py uses), still computed on the raw expansion before
cap_to_max_dim, and corrected the docstring. Also makes a nearby --max_dim
comment dataset-agnostic.

Verified on H2O over 8 GPU ranks, main and the fix back to back: identical
energy (-76.2421767512) and final subspace (1742x1742), stopping on closure
at round 2 instead of on energy convergence at round 3. The expansion there
is 1720 strings against 1742 solved -- a strict subset, adding nothing,
which is exactly what equality could never detect.
@hfwen0502
Sophia Wen (hfwen0502) merged commit df74900 into main Sep 21, 2026
11 checks passed
@hfwen0502
Sophia Wen (hfwen0502) deleted the fix-enlarge-expansion-bugs branch September 21, 2026 18:18
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