Skip to content

Commit de20370

Browse files
authored
Fix bit_length=64 undefined behavior, make it configurable, and lower --samples/--max_nb defaults (#12)
* Fix undefined behavior from hardcoded bit_length=64; make it configurable SBD packs determinant bitstrings into std::vector<size_t> with `bit_length` bits per word. The Python wrapper hardcoded 64 in four places, but bitadvance() in framework/bit_manipulation.h computes size_t d = (((size_t) 1) << bit_length) - 1; Shifting a 64-bit size_t by 64 is undefined behavior. In practice the shift count is masked to 0, so the mask collapses to d == 0 (checked on aarch64 and x86-64). bitadvance() is reached from mpi_redistribution() and mpi_sort_bitarray(), the multi-rank determinant distribution paths. RIKEN documents the default as 20 (see --bit_length in apps/chemistry_tpb_selected_basis_diagonalization/README.md), which is also what run_sbd_diag.py already defaults to; only run_sqd_sbd.py was passing 64. Changes: - Add SBD_DEFAULT_BIT_LENGTH = 20 in sbd_solver.py and use it for the _create_sbd_config default. - Thread the effective value through. _ci_strings_to_sbd_dets() and _sbd_dets_to_ci_strings() now take bit_length, and _solve_sci_core() builds the config before packing so determinants are packed with the same value the C++ engine uses to interpret them. Previously a caller-supplied bit_length reached the engine but not the packing, which would silently corrupt the determinants. - Add --bit_length to run_sqd_sbd.py (default 20) instead of hardcoding. Validated on GB200 (NVHPC 25.5, cc100, 4 GPUs, CUDA-aware MPICH 5.0.1): - H2O 1e-5 subspace, dim 303,601: -76.2437350421 at bit_length 20, both at np=1 and 2x2, matching the pre-change value and run_sbd_diag.py's independent file-based path (-76.2437349413). - bit_length 20 / 48 / 63 give bit-identical energies, confirming that packing, unpacking and the engine now agree. norb=24 at bit_length=20 spans two size_t words, so this also exercises the multi-word path. * Lower --samples default to 3000 and --max_nb default to 10 --max_nb sets the Davidson block size, and davidson_thrust.h allocates 2 * num_block vectors of length dim on the device, so GPU memory scales as dim * (2 * max_nb + ~4) * 8 bytes. 10 matches the upstream default in chemistry/tpb/sbdiag.h and cuts that workspace by ~4x versus 50. * Update example notebook to bit_length 20 The notebook's sbd_config hardcoded 64, the value this PR removes as undefined behavior. Its 'max_nb': 10 already matches the new CLI default.
1 parent 7c7cc09 commit de20370

3 files changed

Lines changed: 36 additions & 13 deletions

File tree

python/examples/run_sqd_sbd.ipynb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@
199199
" 'carryover_type': 1, # singles-only carryover (stable default)\n",
200200
" 'ratio': 0.1,\n",
201201
" 'threshold': 1e-4,\n",
202-
" 'bit_length': 64,\n",
202+
" 'bit_length': 20, # bits per size_t; RIKEN default (64 is UB in bitadvance)\n",
203203
" # serial: 1×1×1 MPI sub-communicator grid\n",
204204
" 'adet_comm_size': 1, 'bdet_comm_size': 1, 'task_comm_size': 1,\n",
205205
"}\n",

python/examples/run_sqd_sbd.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ def parse_args():
6565
p.add_argument("--fcidump", required=True, help="Path to FCIDUMP file")
6666
p.add_argument("--counts", default=None,
6767
help="Path to count_dict.json (bitstring counts from hardware)")
68-
p.add_argument("--samples", type=int, default=10000,
68+
p.add_argument("--samples", type=int, default=3000,
6969
help="Number of uniform random samples (used when --counts is not given)")
7070
p.add_argument("--device",
7171
choices=["auto", "cpu", "gpu", "gpu-omp", "gpu-nvidia-omp"],
@@ -85,13 +85,17 @@ def parse_args():
8585
p.add_argument("--tolerance", "--eps", type=float, default=1e-8, dest="eps")
8686
p.add_argument("--iteration", "--max_it", type=int, default=100, dest="max_it",
8787
help="Max SBD Davidson iterations per diagonalization")
88-
p.add_argument("--block", "--max_nb", type=int, default=50, dest="max_nb")
88+
p.add_argument("--block", "--max_nb", type=int, default=10, dest="max_nb")
8989
p.add_argument("--rdm", "--do_rdm", type=int, default=0, dest="do_rdm",
9090
help="0=density only (default, sufficient for SQD), 1=full RDM")
9191
p.add_argument("--shuffle", "--do_shuffle", type=int, default=0, dest="do_shuffle")
9292
p.add_argument("--carryover_type", type=int, default=1)
9393
p.add_argument("--carryover_ratio", "--ratio", type=float, default=0.1, dest="ratio")
9494
p.add_argument("--carryover_threshold", "--threshold", type=float, default=1e-4, dest="threshold")
95+
p.add_argument("--bit_length", type=int, default=20,
96+
help="Bits packed into each size_t of the bitstring representation "
97+
"(RIKEN default 20). Must be <= 63: bitadvance() shifts a "
98+
"64-bit size_t by this amount, so 64 is undefined behavior.")
9599

96100
# MPI sub-communicator sizes
97101
p.add_argument("--adet_comm_size", type=int, default=1)
@@ -223,7 +227,7 @@ def main():
223227
"carryover_type": args.carryover_type,
224228
"ratio": args.ratio,
225229
"threshold": args.threshold,
226-
"bit_length": 64,
230+
"bit_length": args.bit_length,
227231
"adet_comm_size": args.adet_comm_size,
228232
"bdet_comm_size": args.bdet_comm_size,
229233
"task_comm_size": args.task_comm_size,

python/sbd_solver.py

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,17 @@
2929
import numpy as np
3030
from mpi4py import MPI
3131

32+
# Bits packed into each size_t of SBD's ``std::vector<size_t>`` bitstring
33+
# representation. RIKEN's documented default is 20 (see the --bit_length option
34+
# in apps/chemistry_tpb_selected_basis_diagonalization/README.md), which is also
35+
# what run_sbd_diag.py defaults to.
36+
#
37+
# This must stay <= 63. bitadvance() in framework/bit_manipulation.h computes
38+
# size_t d = (((size_t) 1) << bit_length) - 1;
39+
# so bit_length == 64 shifts a 64-bit size_t by 64, which is undefined behavior;
40+
# in practice the shift count is masked to 0 and the mask collapses to d == 0.
41+
SBD_DEFAULT_BIT_LENGTH = 20
42+
3243
try:
3344
from pyscf import tools as pyscf_tools
3445
except ImportError:
@@ -154,11 +165,15 @@ def _solve_sci_core(
154165
the FCIDUMP only once and reuse it across all batches.
155166
"""
156167
strings_a, strings_b = ci_strings
157-
adet = _ci_strings_to_sbd_dets(strings_a, norb, backend)
158-
bdet = _ci_strings_to_sbd_dets(strings_b, norb, backend)
159168

169+
# Build the config first: it carries the effective bit_length (possibly
170+
# overridden by the caller), and the determinants must be packed with the
171+
# same value the C++ engine will use to interpret them.
160172
sbd_data = _create_sbd_config(sbd_config, backend, device_config)
161173

174+
adet = _ci_strings_to_sbd_dets(strings_a, norb, backend, sbd_data.bit_length)
175+
bdet = _ci_strings_to_sbd_dets(strings_b, norb, backend, sbd_data.bit_length)
176+
162177
# Use .bin extension to trigger SBD's fast binary write path
163178
# (SaveMatrixFormWF in restart.h checks extension: .bin -> raw doubles)
164179
wf_dump_file = sbd_dir / "wavefunction.bin"
@@ -199,8 +214,12 @@ def _solve_sci_core(
199214
occupancies_b = density[1::2]
200215
occupancies = (occupancies_a, occupancies_b)
201216

202-
co_strings_a = _sbd_dets_to_ci_strings(results["carryover_adet"], norb, backend)
203-
co_strings_b = _sbd_dets_to_ci_strings(results["carryover_bdet"], norb, backend)
217+
co_strings_a = _sbd_dets_to_ci_strings(
218+
results["carryover_adet"], norb, backend, sbd_data.bit_length
219+
)
220+
co_strings_b = _sbd_dets_to_ci_strings(
221+
results["carryover_bdet"], norb, backend, sbd_data.bit_length
222+
)
204223

205224
# Read wavefunction coefficients from binary dump
206225
n_alpha_co = len(co_strings_a)
@@ -411,14 +430,14 @@ def _read_fcidump_ecore(fcidump_path):
411430

412431

413432
def _ci_strings_to_sbd_dets(
414-
ci_strings: np.ndarray, norb: int, backend
433+
ci_strings: np.ndarray, norb: int, backend,
434+
bit_length: int = SBD_DEFAULT_BIT_LENGTH,
415435
) -> list[list[int]]:
416436
"""Convert CI strings (integers) to SBD determinant format.
417437
418438
Determinants are sorted in canonical order (matching C++ sort_bitarray)
419439
which is required by the GPU Correlation kernel (do_rdm=1).
420440
"""
421-
bit_length = 64
422441
dets = []
423442
for ci_str in ci_strings:
424443
binary_str = format(int(ci_str), f'0{norb}b')
@@ -428,10 +447,10 @@ def _ci_strings_to_sbd_dets(
428447

429448

430449
def _sbd_dets_to_ci_strings(
431-
dets: list[list[int]], norb: int, backend
450+
dets: list[list[int]], norb: int, backend,
451+
bit_length: int = SBD_DEFAULT_BIT_LENGTH,
432452
) -> np.ndarray:
433453
"""Convert SBD determinants to CI strings (integers)."""
434-
bit_length = 64
435454
ci_strings = []
436455
for det in dets:
437456
binary_str = backend.makestring(det, bit_length, norb)
@@ -459,7 +478,7 @@ def _create_sbd_config(config_dict: dict | None = None, backend=None, device_con
459478
sbd_data.carryover_type = 1
460479
sbd_data.ratio = 0.1
461480
sbd_data.threshold = 1e-4
462-
sbd_data.bit_length = 64
481+
sbd_data.bit_length = SBD_DEFAULT_BIT_LENGTH
463482

464483
if config_dict:
465484
for key, value in config_dict.items():

0 commit comments

Comments
 (0)