From 3f1a9c90463953336a529b1d3a124519f53c4c52 Mon Sep 17 00:00:00 2001 From: Sophia Wen Date: Wed, 30 Sep 2026 16:37:35 -0400 Subject: [PATCH 1/4] Bind tpb::SBD::seed, so a random initial vector is actually controllable 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) --- python/bindings.cpp | 2 + test/test_config_bindings.py | 136 +++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 test/test_config_bindings.py diff --git a/python/bindings.cpp b/python/bindings.cpp index 5aa3c3b..8a93126 100644 --- a/python/bindings.cpp +++ b/python/bindings.cpp @@ -202,6 +202,8 @@ PYBIND11_MODULE(SBD_MODULE_NAME, m) { "Maximum time in seconds") .def_readwrite("init", &sbd::tpb::SBD::init, "Initialization method") + .def_readwrite("seed", &sbd::tpb::SBD::seed, + "Seed for the random initial vector (init = 1)") .def_readwrite("do_shuffle", &sbd::tpb::SBD::do_shuffle, "Shuffle determinants flag") .def_readwrite("do_rdm", &sbd::tpb::SBD::do_rdm, diff --git a/test/test_config_bindings.py b/test/test_config_bindings.py new file mode 100644 index 0000000..e378704 --- /dev/null +++ b/test/test_config_bindings.py @@ -0,0 +1,136 @@ +# This code is a Qiskit project. +# +# (C) Copyright IBM 2026. +# +# This code is licensed under the Apache License, Version 2.0. You may +# obtain a copy of this license in the LICENSE.txt file in the root directory +# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +# +# Any modifications or derivative works of this code must retain this +# copyright notice, and modified files need to carry a notice indicating +# that they have been altered from the originals. + +"""Check that config-struct fields reach the solver, not just the Python object. + +A ``def_readwrite`` that round-trips proves only that the member pointer compiles. +Whether the value changes anything is a separate question, and the interesting failure +is a field that is bound, stores what you set, and is then ignored. + +``seed`` is the case in point: it only matters when ``init == 1`` (random initial +vector), so a test that never sets ``init`` would pass against a binding that does +nothing. The probe is SBD's own ``Davidson iteration 0.0`` line, whose energy is the +Rayleigh quotient of the starting vector -- change the seed and it must move. + +Two practical notes. That line is printed by the C++ layer to file descriptor 1, which +``contextlib.redirect_stdout`` does not capture, so these run as subprocesses. And a +random start needs far more Davidson iterations than the default one, so this uses a +small 24-alpha subspace rather than the full 275-alpha reference case -- the same +anchor ``test_gdb_equivalence.py`` uses, which keeps the whole file to a few seconds. +""" + +from __future__ import annotations + +import pathlib +import re +import subprocess +import sys + +import pytest + +import sbd + +REPO_ROOT = pathlib.Path(__file__).resolve().parents[1] +H2O_DIR = REPO_ROOT / "vendor" / "sbd-upstream" / "data" / "h2o" + +# 24 alpha strings -> a 24^2 = 576-determinant product space. TPB and GDB agree on this +# subspace at -76.0588897208 (see test_gdb_equivalence.py), so it is already pinned. +ALPHA_LIMIT = 24 +ANCHOR_ENERGY = -76.0588897208 + +_RANDOM_START = """ +import pathlib, sys, sbd +backend = sbd.get_backend("cpu") +config = backend.TPB_SBD() +config.eps, config.max_it, config.bit_length = 1e-10, 200, 64 +config.init = 1 # random initial vector -- what seed affects +config.seed = int(sys.argv[2]) +result = sbd.tpb_diag_from_files(sys.argv[1], sys.argv[3], config) +print("FINAL %.10f" % result["energy"]) +""" + +_cache: dict[int, tuple[str, float]] = {} + + +@pytest.fixture(scope="module") +def small_alpha_file(tmp_path_factory): + """The first ALPHA_LIMIT alpha strings, so a random start converges quickly.""" + source = H2O_DIR / "h2o-1em3-alpha.txt" + if not source.is_file(): + pytest.skip(f"vendored h2o data not found at {source}") + lines = [line for line in source.read_text().splitlines() if line.strip()] + path = tmp_path_factory.mktemp("alpha") / "alpha_small.txt" + path.write_text("\n".join(lines[:ALPHA_LIMIT]) + "\n") + return path + + +def _random_start(seed: int, alpha_file: pathlib.Path) -> tuple[str, float]: + """Run a random-start TPB diagonalization; return (iteration-0 line, energy). + + Cached per seed: the reproducibility check re-uses a seed, and there is no point + paying for the same run twice. + """ + if seed in _cache: + return _cache[seed] + completed = subprocess.run( + [sys.executable, "-c", _RANDOM_START, + str(H2O_DIR / "fcidump.txt"), str(seed), str(alpha_file)], + cwd=REPO_ROOT, capture_output=True, text=True, timeout=1800, + ) + assert completed.returncode == 0, ( + f"run failed:\n{completed.stdout[-2000:]}\n{completed.stderr[-1000:]}" + ) + first = re.search( + r"Davidson iteration 0\.0 \(tol=[0-9.e+-]+\): *(-?\d+\.?\d*)", completed.stdout + ) + assert first, f"no iteration-0 line to compare:\n{completed.stdout[-2000:]}" + energy = re.search(r"FINAL (-?\d+\.\d+)", completed.stdout) + assert energy, f"no final energy:\n{completed.stdout[-2000:]}" + _cache[seed] = (first.group(0), float(energy.group(1))) + return _cache[seed] + + +def test_tpb_seed_round_trips(): + """The attribute exists and stores what it is given.""" + config = sbd.get_backend("cpu").TPB_SBD() + config.seed = 4242 + assert config.seed == 4242 + + +def test_tpb_seed_changes_the_starting_vector(small_alpha_file): + """Two seeds must give different start vectors but the same answer. + + The first assertion is what proves the binding reaches ``BasisInitVector``; the + second is what makes it safe -- wherever the solver starts, it must converge to + the same eigenvalue. + """ + first_start, first_energy = _random_start(1729, small_alpha_file) + other_start, other_energy = _random_start(987654321, small_alpha_file) + + assert first_start != other_start, ( + "different seeds produced an identical starting vector, so seed is bound but " + f"not reaching the solver: {first_start}" + ) + assert first_energy == pytest.approx(other_energy, abs=1e-8) + assert first_energy == pytest.approx(ANCHOR_ENERGY, abs=1e-8) + + +def test_tpb_seed_is_reproducible(small_alpha_file): + """The same seed twice must be identical, or the test above proves nothing. + + Without this, two differing start vectors could be run-to-run nondeterminism + rather than the seed taking effect. + """ + start, _ = _random_start(1729, small_alpha_file) + _cache.pop(1729) # force a genuinely fresh run + again, _ = _random_start(1729, small_alpha_file) + assert start == again From a486ef06cf1b54c225544e549c69c5697a23658c Mon Sep 17 00:00:00 2001 From: Sophia Wen Date: Wed, 30 Sep 2026 16:42:22 -0400 Subject: [PATCH 2/4] run_sbd_diag: add --seed, so --init 1 is usable from the driver 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) --- examples/tpb/run_sbd_diag.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/examples/tpb/run_sbd_diag.py b/examples/tpb/run_sbd_diag.py index 4ac49cf..bf95c38 100644 --- a/examples/tpb/run_sbd_diag.py +++ b/examples/tpb/run_sbd_diag.py @@ -107,7 +107,12 @@ def parse_args(): # Initialization and options parser.add_argument('--init', type=int, default=0, - help='Initialization method') + help='Initialization method. 1 selects a random starting ' + 'vector, which is the only mode --seed affects') + parser.add_argument('--seed', type=int, default=1729, + help='Seed for the random starting vector (--init 1). Vary it ' + 'to check whether a result depends on where the solver ' + 'started; the converged energy should not') parser.add_argument('--shuffle', '--do_shuffle', type=int, default=0, dest='do_shuffle', help='Shuffle determinants loaded from --adetfile before ' 'mirroring/deriving beta from them (0=no, 1-4=yes, ' @@ -186,6 +191,7 @@ def main(): config.max_nb = args.max_nb config.max_time = args.max_time config.init = args.init + config.seed = args.seed config.do_shuffle = args.do_shuffle config.do_rdm = 1 if args.rdm_output else 0 config.bit_length = args.bit_length From cd73ba8041581302670b9b3090898d62937d9483 Mon Sep 17 00:00:00 2001 From: Sophia Wen Date: Wed, 30 Sep 2026 16:46:18 -0400 Subject: [PATCH 3/4] Drop test_config_bindings.py 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) --- test/test_config_bindings.py | 136 ----------------------------------- 1 file changed, 136 deletions(-) delete mode 100644 test/test_config_bindings.py diff --git a/test/test_config_bindings.py b/test/test_config_bindings.py deleted file mode 100644 index e378704..0000000 --- a/test/test_config_bindings.py +++ /dev/null @@ -1,136 +0,0 @@ -# This code is a Qiskit project. -# -# (C) Copyright IBM 2026. -# -# This code is licensed under the Apache License, Version 2.0. You may -# obtain a copy of this license in the LICENSE.txt file in the root directory -# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. -# -# Any modifications or derivative works of this code must retain this -# copyright notice, and modified files need to carry a notice indicating -# that they have been altered from the originals. - -"""Check that config-struct fields reach the solver, not just the Python object. - -A ``def_readwrite`` that round-trips proves only that the member pointer compiles. -Whether the value changes anything is a separate question, and the interesting failure -is a field that is bound, stores what you set, and is then ignored. - -``seed`` is the case in point: it only matters when ``init == 1`` (random initial -vector), so a test that never sets ``init`` would pass against a binding that does -nothing. The probe is SBD's own ``Davidson iteration 0.0`` line, whose energy is the -Rayleigh quotient of the starting vector -- change the seed and it must move. - -Two practical notes. That line is printed by the C++ layer to file descriptor 1, which -``contextlib.redirect_stdout`` does not capture, so these run as subprocesses. And a -random start needs far more Davidson iterations than the default one, so this uses a -small 24-alpha subspace rather than the full 275-alpha reference case -- the same -anchor ``test_gdb_equivalence.py`` uses, which keeps the whole file to a few seconds. -""" - -from __future__ import annotations - -import pathlib -import re -import subprocess -import sys - -import pytest - -import sbd - -REPO_ROOT = pathlib.Path(__file__).resolve().parents[1] -H2O_DIR = REPO_ROOT / "vendor" / "sbd-upstream" / "data" / "h2o" - -# 24 alpha strings -> a 24^2 = 576-determinant product space. TPB and GDB agree on this -# subspace at -76.0588897208 (see test_gdb_equivalence.py), so it is already pinned. -ALPHA_LIMIT = 24 -ANCHOR_ENERGY = -76.0588897208 - -_RANDOM_START = """ -import pathlib, sys, sbd -backend = sbd.get_backend("cpu") -config = backend.TPB_SBD() -config.eps, config.max_it, config.bit_length = 1e-10, 200, 64 -config.init = 1 # random initial vector -- what seed affects -config.seed = int(sys.argv[2]) -result = sbd.tpb_diag_from_files(sys.argv[1], sys.argv[3], config) -print("FINAL %.10f" % result["energy"]) -""" - -_cache: dict[int, tuple[str, float]] = {} - - -@pytest.fixture(scope="module") -def small_alpha_file(tmp_path_factory): - """The first ALPHA_LIMIT alpha strings, so a random start converges quickly.""" - source = H2O_DIR / "h2o-1em3-alpha.txt" - if not source.is_file(): - pytest.skip(f"vendored h2o data not found at {source}") - lines = [line for line in source.read_text().splitlines() if line.strip()] - path = tmp_path_factory.mktemp("alpha") / "alpha_small.txt" - path.write_text("\n".join(lines[:ALPHA_LIMIT]) + "\n") - return path - - -def _random_start(seed: int, alpha_file: pathlib.Path) -> tuple[str, float]: - """Run a random-start TPB diagonalization; return (iteration-0 line, energy). - - Cached per seed: the reproducibility check re-uses a seed, and there is no point - paying for the same run twice. - """ - if seed in _cache: - return _cache[seed] - completed = subprocess.run( - [sys.executable, "-c", _RANDOM_START, - str(H2O_DIR / "fcidump.txt"), str(seed), str(alpha_file)], - cwd=REPO_ROOT, capture_output=True, text=True, timeout=1800, - ) - assert completed.returncode == 0, ( - f"run failed:\n{completed.stdout[-2000:]}\n{completed.stderr[-1000:]}" - ) - first = re.search( - r"Davidson iteration 0\.0 \(tol=[0-9.e+-]+\): *(-?\d+\.?\d*)", completed.stdout - ) - assert first, f"no iteration-0 line to compare:\n{completed.stdout[-2000:]}" - energy = re.search(r"FINAL (-?\d+\.\d+)", completed.stdout) - assert energy, f"no final energy:\n{completed.stdout[-2000:]}" - _cache[seed] = (first.group(0), float(energy.group(1))) - return _cache[seed] - - -def test_tpb_seed_round_trips(): - """The attribute exists and stores what it is given.""" - config = sbd.get_backend("cpu").TPB_SBD() - config.seed = 4242 - assert config.seed == 4242 - - -def test_tpb_seed_changes_the_starting_vector(small_alpha_file): - """Two seeds must give different start vectors but the same answer. - - The first assertion is what proves the binding reaches ``BasisInitVector``; the - second is what makes it safe -- wherever the solver starts, it must converge to - the same eigenvalue. - """ - first_start, first_energy = _random_start(1729, small_alpha_file) - other_start, other_energy = _random_start(987654321, small_alpha_file) - - assert first_start != other_start, ( - "different seeds produced an identical starting vector, so seed is bound but " - f"not reaching the solver: {first_start}" - ) - assert first_energy == pytest.approx(other_energy, abs=1e-8) - assert first_energy == pytest.approx(ANCHOR_ENERGY, abs=1e-8) - - -def test_tpb_seed_is_reproducible(small_alpha_file): - """The same seed twice must be identical, or the test above proves nothing. - - Without this, two differing start vectors could be run-to-run nondeterminism - rather than the seed taking effect. - """ - start, _ = _random_start(1729, small_alpha_file) - _cache.pop(1729) # force a genuinely fresh run - again, _ = _random_start(1729, small_alpha_file) - assert start == again From 0ffaf286f05c57210c06656c5e7f01291d83c715 Mon Sep 17 00:00:00 2001 From: Sophia Wen Date: Wed, 30 Sep 2026 16:57:32 -0400 Subject: [PATCH 4/4] run_sbd_diag: say what --init actually does 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) --- examples/tpb/run_sbd_diag.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/examples/tpb/run_sbd_diag.py b/examples/tpb/run_sbd_diag.py index bf95c38..109c55a 100644 --- a/examples/tpb/run_sbd_diag.py +++ b/examples/tpb/run_sbd_diag.py @@ -107,12 +107,15 @@ def parse_args(): # Initialization and options parser.add_argument('--init', type=int, default=0, - help='Initialization method. 1 selects a random starting ' - 'vector, which is the only mode --seed affects') + help='Starting vector. 0 puts unit weight on the FIRST ' + 'determinant of the basis -- which is Hartree-Fock only ' + 'if the basis happens to be ordered that way. 1 is random. ' + 'Nothing else is implemented, and other values start from ' + 'a zero vector without complaining') parser.add_argument('--seed', type=int, default=1729, - help='Seed for the random starting vector (--init 1). Vary it ' - 'to check whether a result depends on where the solver ' - 'started; the converged energy should not') + help='Seed for --init 1, ignored otherwise (default matches ' + "upstream's). Vary it to check whether a result depends " + 'on where the solver started; the energy should not') parser.add_argument('--shuffle', '--do_shuffle', type=int, default=0, dest='do_shuffle', help='Shuffle determinants loaded from --adetfile before ' 'mirroring/deriving beta from them (0=no, 1-4=yes, '