Duplicate-determinant handling differs between TPB and GDB; cost of the #29 label conversion at scale

Read framework/bit_manipulation.h:154 and :386, python/bindings.cpp:477-485, and the TPB call sites around _sbd_dets_to_ci_strings. First determine whether TPB and GDB should share duplicate handling, then profile the conversion on the listed inputs. Done means the policy is explicit and any pursued conversion improvement is validated without weakening #29’s label consistency.

Two points from reviewing #29 that did not block it, but are worth recording. Both still hold on main at 6721877.

1. TPB and GDB now answer the duplicate-determinant question in opposite ways

sort_bitarray (framework/bit_manipulation.h:386) both sorts and deduplicates:

std::sort(a.begin(), a.end(), [](...){ return less_from_back(x,y); });
a.erase(std::unique(a.begin(), a.end()), a.end());

The GDB binding treats a shrink as a caller error, with a stated rationale (python/bindings.cpp:477-485):

// sort_bitarray also removes duplicates, which would leave
// part of the subspace unreachable, so reject them instead of
// dropping them silently.
...
throw std::invalid_argument("gdb_diag requires distinct determinants");

After #29 the TPB path takes the other branch: duplicates are dropped, the labels are derived from adet/bdet so they stay consistent with the amplitudes, and the caller is returned a smaller subspace than it asked for without notice.

#29's choice looks right to me — sort_bitarray's documented API permits duplicates, so a caller following it should not hit an exception, and deriving labels from the diagonalized lists is the property worth having. But the repo should not hold both answers silently. Either:

• TPB is correct and the GDB check should be reconsidered, or
• the difference is deliberate (GDB's single-list subspace really does become unreachable in a way TPB's tensor product does not) and a comment on the TPB path should say so.

Worth noting the GDB comment's concern is not mislabeling — it is the caller receiving a smaller subspace than requested. Post-#29 that outcome is silent on the TPB path. Probably acceptable, but it is a behavior change beyond "the size check no longer misfires on benign dedup," and it is not reachable from run_sqd_sbd.py (fermion.py dedupes with np.unique first), so only direct solve_sci / solve_sci_batch callers see it.

2. _sbd_dets_to_ci_strings is O(n) Python on the control process, called twice per diagonalization

#29 added:

solved_strings_a = _sbd_dets_to_ci_strings(adet, norb, backend, sbd_data.bit_length)
solved_strings_b = _sbd_dets_to_ci_strings(bdet, norb, backend, sbd_data.bit_length)

The helper is a per-determinant Python loop doing backend.makestring then int(binary_str, 2). At the bundled H2O case (275 strings) this is irrelevant. At the larger vendored inputs it is not: data/h2o/h2o-1em8-alpha.txt is 39,028 alpha strings, data/c4h4 14,690, data/nh3 118,755. That is tens of thousands of string-format-and-parse round trips per call, per spin sector, on rank 0 while every other rank waits.

Not a blocker and not worth trading away #29's correctness property — comparing len(adet) against len(strings_a) and converting only on mismatch would reintroduce exactly the "two lists expected to agree" coupling #29 removed. Recording it because it scales with the inputs we intend to run, and a vectorised conversion (or one pushed into the binding) would be a contained change if it ever shows up in a profile.

A note on #29's ordering argument

#29 describes the agreement between sort_bitarray's canonical order and ascending-integer order as a coincidence established by measurement. It is stronger than that: less_from_back (bit_manipulation.h:154) compares words from the highest index downward, i.e. most-significant word first, and from_string packs a big-endian bitstring so the most significant bits land in the last word. For fixed-width determinants that is ascending integer order, structurally.

This does not weaken the case for #29 — nothing contracts the packing either, and deriving labels from adet/bdet is the right fix regardless. But the measurement was confirming a consequence of the packing rather than a happy accident, which is worth knowing if anyone revisits this.

This issue was generated by Claude Opus 5 under my guidance.