Bind tpb::SBD::seed, so a random initial vector is actually controllable - #48
Merged
Merged
Conversation
We expose `init`, and `init == 1` means "random start vector" (tpb/sbdiag.h:312 passes it to BasisInitVector along with `seed`), but `seed` itself was never bound -- so every random-start TPB run was pinned to upstream's default 1729 with no way to vary it. That defeats the one thing a random start is for: checking whether a result depends on where the solver began. GDB has bound `seed` all along, so this was an asymmetry between the two config structs rather than a decision. Two lines, mirroring GDB's entry, with the docstring noting it only applies at init = 1 -- which the field name does not convey. test/test_config_bindings.py is new, and deliberately does not stop at a round-trip: a field that stores what you set and is then ignored would pass that. It compares SBD's own "Davidson iteration 0.0" line, whose energy is the Rayleigh quotient of the starting vector, across two seeds -- 1729 gives tol=2.71579 where 987654321 gives tol=2.70452 on the full case -- and asserts the same seed reproduces exactly while both converge to the same eigenvalue. Verified the assertion discriminates: forcing both runs to one seed makes the start vectors identical and the test fails, so it catches "bound but ignored" rather than only "not bound". Runs on a 24-alpha subspace against the -76.0588897208 anchor rather than the full 275-alpha reference case. A random start needs many more Davidson iterations than the default one, and the full case took 133 s where this takes 10 s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The driver already exposed --init, so you could ask for a random starting vector but not choose which one -- the same gap the binding fix closes, one layer up. Without this the fix is reachable only by writing Python against the binding. Also says in --init's help that 1 is the mode --seed affects, since neither name conveys that on its own. Verified end to end on a 24-alpha h2o subspace: --seed 1729 starts at tol=1.11868 / -73.3618 and --seed 987654321 at tol=1.14711 / -73.2877, both converging to -76.0588897208. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Too much test surface for a two-line config binding. The behaviour was verified by hand instead and the evidence is in the previous commits and the PR description: --seed 1729 starts at tol=2.71579 where 987654321 starts at tol=2.70452, the same seed reproduces exactly, and both converge to -76.2359466308. What is given up is the regression guard. If someone later removes the binding or upstream stops honouring `seed`, nothing fails -- the field would keep storing what you set while being ignored, which is the failure mode the deleted test was built to catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Upstream documents init==0 as "the HF solution / start from fermi sea" (tpb/davidson.h:50), but the code is `W[0] = 1.0` -- unit weight on the first determinant of the basis, with no search for Hartree-Fock. Identical in GDB (gdb/davidson.h:18). The two coincide only when the basis is ordered so HF sorts first, and a subspace of sampled determinants generally is not: that is how a run can start on a high-order excitation with no coupling to the rest and return a diagonal element as its energy. Also notes that only 0 and 1 exist, and that other values start from a zero vector without erroring -- there is no else branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TPB_SBDexposesinitbut notseed. Sinceinit = 1means "random initial vector" —tpb/sbdiag.h:312passes both toBasisInitVector— every random-start TPB run was pinned to upstream's default1729, with no way to vary it. That defeats the point of a random start: checking whether a result depends on where the solver began.GDB_SBDhas boundseedall along, so this was an asymmetry between the two config structs rather than a deliberate omission.The change
Two lines in
python/bindings.cppmirroring GDB's entry, plus--seedonexamples/tpb/run_sbd_diag.py— the driver already had--init, so you could ask for a random start without being able to choose which one. Both help strings now say that--init 1is the mode--seedaffects, since neither name conveys that alone.Verification
Done by hand rather than as a shipped test, to keep the test surface proportionate to a two-line binding.
config.seed = 4242→ reads back4242, maps asinttol=2.71579, -68.5062vstol=2.70452, -68.4933-76.2359466308, matching the pinned referenceThe probe is SBD's own
Davidson iteration 0.0line, whose energy is the Rayleigh quotient of the starting vector — a round-trip check alone would pass against a field that stores what you set and is then ignored. I also confirmed the comparison discriminates: forcing both runs to one seed makes the start vectors identical.Through the driver on a 24-alpha h2o subspace:
--seed 1729starts attol=1.11868 / -73.3618,--seed 987654321attol=1.14711 / -73.2877, both converging to-76.0588897208.4 passedserial,3 passedundermpirun -n 2.Not covered: there is no regression guard. If the binding is later removed, or upstream stops honouring
seed, nothing fails — the field would keep storing what you set while being ignored.Related, not included
initial_adeterminant_bitstring/initial_bdeterminant_bitstring(TPB) andinitial_determinant_bitstring(GDB) are also unbound. Those let you name the starting determinant instead of taking whichever sorts first, and both solvers already consume them (tpb/sbdiag.h:335-338,gdb/sbdiag.h:292-305).GDB_SBD.timing_barriersis unbound too. Each is its own change.Separately worth an upstream issue:
max_timeis narrower than it looks — GDB's CPU Davidson overloads (gdb/davidson.h:41,:317) do not take it, andgdb/sbdiag.hpasses it only at:370inside the Thrust branch, so it is inert on_core_cpuand_core_gpu_omp_offload.🤖 Generated with Claude Code