From 7c08ef0606e9b1c5ef61dc2bba216f83188d1d97 Mon Sep 17 00:00:00 2001 From: Jim Garrison Date: Wed, 21 Oct 2026 22:30:33 -0400 Subject: [PATCH 1/3] Enable a minimum set of linters sbd-eigensolver-python is the only Python repo among the addons with no linter running anywhere: `lint` was absent from CI and tox.ini defined neither a lint nor a style env. This adds the smallest set worth having. Three tools, chosen for what they cost. `ruff check` is the one every other addon runs, with the same `select` block and the same ruff pin as qiskit-addon-sqd and qiskit-fermions. The copyright-header check and `reno lint` both pass on the tree as it stands -- all sixteen Python files already carry the header -- so they add enforcement without a diff. Each is a regression guard from here on rather than a cleanup. Deliberately left out: `ruff format`, which would touch nearly every line and bury the rest of this change; mypy, whose six errors are all genuine typing decisions about an extension with no stubs, and which deserves its own pass; and pylint, which at seven violations adds little next to ruff. The linters live in `[dependency-groups]` rather than extras, since the lint envs set `skip_install` and never install the package alongside them. That also means neither env compiles the extension, so both run on a machine with no MPI or BLAS. `dependency_groups` needs tox 4.22, so minversion moves from 4.4.4 to 4.28.0, matching qiskit-fermions. isort gets `known-first-party = ["sbd"]`. The package directory is `python/` but it imports as `sbd`, which isort cannot infer, and without it `import sbd` groups with the third-party imports in five test files. Four per-file ignores, each with its reason in the config. Two are worth naming here: `l` in sbd_solver.py is the FCIDUMP two-electron integral index order, and `__all__` is grouped by topic with comments that sorting would flatten. The examples keep their mathematical symbols and their read-top-to-bottom comments in place of docstrings. One real bug turned up in the first run: `init()` and `_load_backend()` both discarded the original exception, so a missing-mpi4py traceback lost its cause. Both now raise `from`. The rest of the source changes are mechanical: import sorting, trailing whitespace, f-strings with no placeholders, a duplicate name in `__all__`, an unused `Sequence` import, `Callable` moved to `collections.abc`, `zip(x, x[1:])` to `itertools.pairwise`, two unused unpacked variables, `try/except/pass` to `contextlib.suppress`, and four docstring fixes. Verified with `tox -e py314` (51 passed, 35 skipped), `tox -e mpi` under two ranks (23 passed, 63 skipped), `tox -e lint` and `tox -e docs`. Assisted-by: Claude Opus 5 --- .github/workflows/lint.yml | 35 ++++++++ examples/gdb/run_gdb_diag.py | 3 +- examples/gdb/run_gdb_heatbath.py | 6 +- examples/tpb/run_sbd_diag.py | 32 +++---- examples/tpb/run_sqd_enlarge_subspace_sbd.py | 7 +- examples/tpb/run_sqd_sbd.ipynb | 10 ++- examples/tpb/run_sqd_sbd.py | 2 +- pyproject.toml | 86 +++++++++++++++++++ python/__init__.py | 45 ++++------ python/device_config.py | 41 ++++----- python/sbd_solver.py | 23 ++--- .../enable-linters-77600380da49ab67.yaml | 16 ++++ test/test_gdb_drivers.py | 3 +- test/test_gdb_equivalence.py | 6 +- tox.ini | 28 +++++- 15 files changed, 246 insertions(+), 97 deletions(-) create mode 100644 .github/workflows/lint.yml create mode 100644 releasenotes/notes/enable-linters-77600380da49ab67.yaml diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..d6c332e --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,35 @@ +name: Lint check + +on: + push: + branches: + - main + - 'stable/**' + pull_request: + branches: + - main + - 'stable/**' + +jobs: + lint: + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@v7 + with: + # reno reads the notes from git history, so `reno lint` needs more than a + # shallow clone. The submodule the tests need is not checked out: the lint + # environment sets `skip_install`, so nothing here compiles the extension. + fetch-depth: 0 + - name: Set up Python 3.11 + uses: actions/setup-python@v7 + with: + python-version: '3.11' + - name: Install tox + run: | + python -m pip install --upgrade pip + pip install tox + - name: Run lint check + shell: bash + run: | + tox -elint diff --git a/examples/gdb/run_gdb_diag.py b/examples/gdb/run_gdb_diag.py index 8c72b30..ea02cb8 100755 --- a/examples/gdb/run_gdb_diag.py +++ b/examples/gdb/run_gdb_diag.py @@ -12,8 +12,7 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. -""" -Standalone GDB diagonalization over an explicit determinant list, in memory. +"""Standalone GDB diagonalization over an explicit determinant list, in memory. GDB (general determinant basis) spans the subspace with the determinants it is given. TPB spans it with the Cartesian product of an alpha and a beta list, so diff --git a/examples/gdb/run_gdb_heatbath.py b/examples/gdb/run_gdb_heatbath.py index 2f9b744..9063a67 100755 --- a/examples/gdb/run_gdb_heatbath.py +++ b/examples/gdb/run_gdb_heatbath.py @@ -12,8 +12,7 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. -""" -Grow a GDB subspace with SBD's own heatbath expansion, round after round. +"""Grow a GDB subspace with SBD's own heatbath expansion, round after round. This is selected CI (HCI) driven from Python: diagonalize, let SBD expand the subspace from the resulting wavefunction, diagonalize the larger subspace, repeat. @@ -403,9 +402,10 @@ def reshard(det, args, comm, rank, size): def main(): args = parse_args() - import sbd from mpi4py import MPI + import sbd + sbd.init(device=args.device) backend = sbd.get_backend(args.device) comm = MPI.COMM_WORLD diff --git a/examples/tpb/run_sbd_diag.py b/examples/tpb/run_sbd_diag.py index 109c55a..59efcff 100644 --- a/examples/tpb/run_sbd_diag.py +++ b/examples/tpb/run_sbd_diag.py @@ -12,8 +12,7 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. -""" -Standalone SBD diagonalization (no SQD loop, no qiskit dependency). +"""Standalone SBD diagonalization (no SQD loop, no qiskit dependency). Runs a single TPB diagonalization from an FCIDUMP file and alpha determinant file using the SBD library directly. @@ -50,13 +49,14 @@ import numpy as np + def parse_args(): """Parse command line arguments for all TPB_SBD parameters""" parser = argparse.ArgumentParser( description='Quantum chemistry calculation with CPU/GPU support', formatter_class=argparse.ArgumentDefaultsHelpFormatter ) - + # Device selection parser.add_argument('--device', choices=['auto', 'cpu', @@ -68,7 +68,7 @@ def parse_args(): 'gpu-omp (nvc++ OpenMP target offload) | auto. ' 'gpu-nvhpc-omp / gpu-nvidia-omp are deprecated ' 'aliases for gpu-omp.') - + # Input files parser.add_argument('--fcidump', default='../../vendor/sbd-upstream/data/h2o/fcidump.txt', help='Path to FCIDUMP file') @@ -84,7 +84,7 @@ def parse_args(): help='Load initial wavefunction from file') parser.add_argument('--savename', default='', help='Save final wavefunction to file') - + # MPI communicator sizes parser.add_argument('--adet_comm_size', type=int, default=1, help='Alpha determinant communicator size') @@ -92,7 +92,7 @@ def parse_args(): help='Beta determinant communicator size') parser.add_argument('--task_comm_size', type=int, default=1, help='Helper communicator size') - + # Diagonalization method and convergence parser.add_argument('--method', type=int, default=0, choices=[0, 1, 2, 3], help='Diagonalization method: 0=Davidson, 1=Davidson+Ham, 2=Lanczos, 3=Lanczos+Ham') @@ -104,7 +104,7 @@ def parse_args(): help='Convergence tolerance') parser.add_argument('--max_time', type=float, default=1e10, help='Maximum time in seconds') - + # Initialization and options parser.add_argument('--init', type=int, default=0, help='Starting vector. 0 puts unit weight on the FIRST ' @@ -147,11 +147,11 @@ def parse_args(): help='Carryover ratio') parser.add_argument('--carryover_threshold', '--threshold', type=float, default=0.0, dest='threshold', help='Carryover threshold') - + # Determinant representation parser.add_argument('--bit_length', type=int, default=20, help='Bit length for determinant representation') - + # Output options parser.add_argument('--dump_matrix_form_wf', default='', help='Filename to dump wavefunction in matrix form') @@ -162,13 +162,13 @@ def parse_args(): parser.add_argument('--max_memory_gb_for_determinants', '--gpu-memory', type=int, default=-1, dest='max_memory_gb_for_determinants', help='Maximum GPU memory in GB for determinants (-1=auto, GPU only)') - + return parser.parse_args() def main(): args = parse_args() - + # Import sbd — auto-initializes on first use, but we call init() # explicitly here to set the default device from --device flag. import sbd @@ -178,14 +178,14 @@ def main(): rank = sbd.get_rank() size = sbd.get_world_size() - + if rank == 0: print("="*70) print("SBD Simplified API - Chemistry Calculation") print("="*70) sbd.print_info() print() - + # Configure calculation config = sbd.TPB_SBD() config.max_it = args.max_it @@ -211,12 +211,12 @@ def main(): config.use_precalculated_dets = bool(args.use_precalculated_dets) if hasattr(config, 'max_memory_gb_for_determinants'): config.max_memory_gb_for_determinants = args.max_memory_gb_for_determinants - + if rank == 0: print("Configuration:") print(f" Device: {sbd.get_device()}") print(f" Communication: {sbd.get_comm_backend()}") - print(f" Method: Davidson") + print(" Method: Davidson") print(f" Max iterations: {config.max_it}") print(f" Tolerance: {config.eps}") print(f" MPI ranks: {size}") @@ -231,7 +231,7 @@ def main(): f"requires task x adet x bdet x helper == ranks exactly, so this " f"will abort in TaskCommunicator with 'MPI Size of twister is not " f"a square of a integer'. Use a multiple of {_grid} ranks.") - print(f"\nInput files:") + print("\nInput files:") print(f" FCIDUMP: {args.fcidump}") print(f" Alpha dets: {args.adetfile}") print() diff --git a/examples/tpb/run_sqd_enlarge_subspace_sbd.py b/examples/tpb/run_sqd_enlarge_subspace_sbd.py index f767cba..4c0dbdf 100644 --- a/examples/tpb/run_sqd_enlarge_subspace_sbd.py +++ b/examples/tpb/run_sqd_enlarge_subspace_sbd.py @@ -59,7 +59,10 @@ from mpi4py import MPI from pyscf import ao2mo, tools from qiskit.primitives import BitArray -from qiskit_addon_sqd.fermion import diagonalize_fermionic_hamiltonian, enlarge_batch_from_transitions +from qiskit_addon_sqd.fermion import ( + diagonalize_fermionic_hamiltonian, + enlarge_batch_from_transitions, +) def parse_args(): @@ -318,8 +321,8 @@ def main(): print(f"Device: {args.device}") print() - from sbd.sbd_solver import solve_sci_batch from sbd.device_config import DeviceConfig + from sbd.sbd_solver import solve_sci_batch device_str = args.device if device_str == "auto": diff --git a/examples/tpb/run_sqd_sbd.ipynb b/examples/tpb/run_sqd_sbd.ipynb index 9353e0b..868c90b 100644 --- a/examples/tpb/run_sqd_sbd.ipynb +++ b/examples/tpb/run_sqd_sbd.ipynb @@ -57,17 +57,19 @@ } ], "source": [ + "import json\n", "from functools import partial\n", - "import json, numpy as np\n", "from pathlib import Path\n", + "\n", + "import numpy as np\n", "from mpi4py import MPI\n", "from pyscf import ao2mo, tools\n", - "from qiskit_addon_sqd.fermion import diagonalize_fermionic_hamiltonian\n", "from qiskit.primitives import BitArray\n", + "from qiskit_addon_sqd.fermion import diagonalize_fermionic_hamiltonian\n", "\n", "import sbd\n", - "from sbd.sbd_solver import solve_sci_batch\n", "from sbd.device_config import DeviceConfig, print_device_info\n", + "from sbd.sbd_solver import solve_sci_batch\n", "\n", "comm = MPI.COMM_WORLD\n", "print(f'SBD version : {sbd.__version__}')\n", @@ -517,7 +519,7 @@ "\n", "print()\n", "print(f'Final SQD energy: {result.energy + nuc:.10f} Ha (electronic {result.energy:.10f} + nuc {nuc:.6f})')\n", - "print(f'FCI reference : ≈ −76.24 Ha')" + "print('FCI reference : ≈ −76.24 Ha')" ] }, { diff --git a/examples/tpb/run_sqd_sbd.py b/examples/tpb/run_sqd_sbd.py index 4b37132..50b2f61 100644 --- a/examples/tpb/run_sqd_sbd.py +++ b/examples/tpb/run_sqd_sbd.py @@ -385,8 +385,8 @@ def main(): # solve_sci_batch auto-initializes the SBD backend on first call and # falls back to MPI.COMM_WORLD when mpi_comm is not provided, so no # explicit sbd.init() / mpi_comm threading is needed in user code. - from sbd.sbd_solver import solve_sci_batch from sbd.device_config import DeviceConfig, print_device_info + from sbd.sbd_solver import solve_sci_batch device_str = args.device if device_str == "auto": diff --git a/pyproject.toml b/pyproject.toml index 8946b29..f405e4f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,6 +63,18 @@ notebook-dependencies = [ "pyscf>=2.9", ] +[dependency-groups] +# The linters live in dependency groups rather than optional-dependencies: they are +# never installed alongside the package (the lint envs set `skip_install`), so there is +# no reason to publish them as extras in the wheel metadata. +style = [ + "ruff==0.16.9", +] +lint = [ + {include-group = "style"}, + "reno>=4.1", +] + [project.urls] Homepage = "https://github.com/Qiskit/sbd-eigensolver-python" Documentation = "https://github.com/Qiskit/sbd-eigensolver-python" @@ -76,6 +88,80 @@ packages = ["sbd"] package-dir = {sbd = "python"} zip-safe = false +[tool.ruff] +line-length = 100 +src = ["python", "test"] +target-version = "py311" + +[tool.ruff.lint] +select = [ + "I", # isort + "E", # pycodestyle + "W", # pycodestyle + "D", # pydocstyle + "F", # pyflakes + "RUF", # ruff + "UP", # pyupgrade + "SIM", # flake8-simplify + "B", # flake8-bugbear + "A", # flake8-builtins +] +ignore = [ + "E501", # line too long +] + +[tool.ruff.lint.extend-per-file-ignores] +"test/**.py" = [ + "D", # pydocstyle +] +"docs/**/*" = [ + "E402", # module level import not at top of file + "D", # pydocstyle +] +# The examples are written to be read top to bottom, so their helpers carry an +# explanatory comment rather than a docstring, and the prose uses the same +# mathematical symbols (alpha, the minus sign, the multiplication sign) as the +# surrounding discussion. +"examples/**/*" = [ + "D", # pydocstyle + "RUF001", # ambiguous unicode character in string + "RUF003", # ambiguous unicode character in comment + "SIM102", # collapsible if: the nesting mirrors the explanation +] +# `i, j, k, l` is the FCIDUMP two-electron integral index order, so `l` here is +# the name the file format uses rather than an ambiguous identifier. +"python/sbd_solver.py" = [ + "E741", # ambiguous variable name +] +# `__all__` is grouped by topic with comments, which sorting would flatten. +"python/__init__.py" = [ + "RUF022", # `__all__` is not sorted +] + +[tool.ruff.lint.isort] +# The package directory is `python/` but it imports as `sbd` (see +# `[tool.setuptools] package-dir`), so isort cannot infer that `sbd` is ours and +# would otherwise group it with the third-party imports. +known-first-party = ["sbd"] + +[tool.ruff.lint.pydocstyle] +convention = "google" + +[tool.ruff.lint.flake8-copyright] +notice-rgx = """ +# This code is a Qiskit project. +# +# \\(C\\) Copyright IBM \\d{4}((,\\s)\\d{4})*\\. +# +# 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\\. +""" + [tool.pytest.ini_options] testpaths = ["test"] # -rs lists the skipped tests and why, which matters here because most of the suite diff --git a/python/__init__.py b/python/__init__.py index e0f1c7e..e80180c 100644 --- a/python/__init__.py +++ b/python/__init__.py @@ -10,8 +10,7 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. -""" -SBD (Selected Basis Diagonalization) Python Bindings +"""SBD (Selected Basis Diagonalization) Python bindings. This package provides Python bindings for the SBD library. @@ -29,10 +28,11 @@ result_gpu = sbd.tpb_diag(..., device='gpu') """ +import contextlib import os import re -from importlib.metadata import version import subprocess +from importlib.metadata import version # Single-sourced from the [project] table in pyproject.toml, so the version is # declared in exactly one place. Note the argument is the *distribution* name @@ -88,7 +88,6 @@ def _extension_path(module_name): """Path of the built extension for `module_name`, or None.""" - import os here = os.path.dirname(os.path.abspath(__file__)) try: names = os.listdir(here) @@ -112,7 +111,8 @@ def _inspect_extension(path): an ABI/symbol mismatch, which only surfaces on a real dlopen; that is reported by _load_backend() at first use. """ - import subprocess, sys as _sys + import subprocess + import sys as _sys if _sys.platform == 'darwin': return True, None # otool output does not mark unresolved deps try: @@ -131,7 +131,7 @@ def _inspect_extension(path): f"host is {platform.machine()}. Rebuild it here. (Note " f"the loader reports this as 'cannot open shared object " f"file', which is misleading -- the file exists.)") - missing = [l.split('=>')[0].strip() for l in out.splitlines() if 'not found' in l] + missing = [ln.split('=>')[0].strip() for ln in out.splitlines() if 'not found' in ln] if missing: return False, (f"missing shared librar{'y' if len(missing) == 1 else 'ies'}: " f"{', '.join(missing)}. Put the containing directory on " @@ -224,7 +224,7 @@ def _load_backend(device): except Exception as exc: # Never substitute a different backend: a silently-swapped device is the # same class of lie as a silent host fallback. - ok, reason = _inspect_extension(path) + _, reason = _inspect_extension(path) detail = reason or f"{type(exc).__name__}: {exc}" _backend_errors[device] = detail raise RuntimeError( @@ -311,8 +311,7 @@ def _resolve_device(device): def init(device='cpu', comm_backend='mpi'): - """ - Initialize SBD with MPI and set the default compute device. + """Initialize SBD with MPI and set the default compute device. Calling ``init()`` explicitly is **optional** — SBD auto-initializes on first use with ``device='cpu'`` and ``comm_backend='mpi'``. Call it @@ -354,10 +353,10 @@ def init(device='cpu', comm_backend='mpi'): _comm_module = MPI _global_comm = MPI.COMM_WORLD _comm_backend = 'mpi' - except ImportError: + except ImportError as exc: raise RuntimeError( "MPI backend requires mpi4py. Install with: pip install mpi4py" - ) + ) from exc else: raise ValueError(f"Unknown comm_backend: '{comm_backend}'. Supported: 'mpi'") @@ -375,8 +374,7 @@ def init(device='cpu', comm_backend='mpi'): def finalize(): - """ - Finalize SBD and reset session state. + """Finalize SBD and reset session state. Synchronizes GPU (if used) but does NOT call MPI_Finalize — mpi4py handles MPI lifecycle automatically. @@ -388,10 +386,8 @@ def finalize(): # Synchronize GPU backends for name, backend in _backends.items(): if name == 'gpu' and hasattr(backend, 'cleanup_device'): - try: + with contextlib.suppress(Exception): backend.cleanup_device() - except Exception: - pass _default_device = None _comm_backend = None @@ -410,8 +406,7 @@ def is_initialized(): # --------------------------------------------------------------------------- def get_backend(device=None): - """ - Get the backend module for the given device. + """Get the backend module for the given device. Auto-initializes SBD if needed. Passing ``device`` overrides the default, which is how you switch backends within one process. The one @@ -647,8 +642,7 @@ def sort_bitarray(dets, device=None): def tpb_diag_from_files(fcidumpfile, adetfile, sbd_data, loadname="", savename="", device=None): - """ - Perform TPB diagonalization from files. + """Perform TPB diagonalization from files. The one determinant file supplies both the alpha and the beta list, so the subspace is the product of that list with itself. For different alpha and @@ -681,8 +675,7 @@ def tpb_diag_from_files(fcidumpfile, adetfile, sbd_data, def tpb_diag(fcidump, adet, bdet, sbd_data, loadname="", savename="", device=None): - """ - Perform TPB diagonalization with data structures. + """Perform TPB diagonalization with data structures. Args: fcidump: FCIDump object. @@ -749,8 +742,7 @@ def gdb_diag(fcidump, det, sbd_data, loadname="", savename="", device=None, determinant_distribution="", determinant_grid_a=0, determinant_grid_b=0): - """ - Perform GDB diagonalization over an explicit list of determinants. + """Perform GDB diagonalization over an explicit list of determinants. Unlike TPB, which spans the subspace with the Cartesian product of an alpha and a beta determinant list, GDB spans it with the given determinants @@ -896,12 +888,12 @@ def print_info(): print(f"Compiled backends: {', '.join(available_backends()) or 'none'}") if _initialized: - print(f"\nCurrent session:") + print("\nCurrent session:") print(f" Default device: {_default_device}") print(f" Communication: {_comm_backend}") print(f" MPI rank: {get_rank()}/{get_world_size()}") else: - print(f"\nNot initialized. Call sbd.init() or any API function to start.") + print("\nNot initialized. Call sbd.init() or any API function to start.") print("=" * 60) @@ -952,7 +944,6 @@ def print_info(): 'gdb_diag', # Utilities - 'available_backends', 'print_info', # Sub-modules diff --git a/python/device_config.py b/python/device_config.py index 477c53a..7b443b1 100644 --- a/python/device_config.py +++ b/python/device_config.py @@ -10,8 +10,7 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. -""" -Device configuration helper for SBD Python bindings. +"""Device configuration helper for SBD Python bindings. This module provides utilities to easily switch between CPU and GPU execution without changing user code. @@ -22,9 +21,8 @@ class DeviceConfig: - """ - Helper class to configure CPU vs GPU execution for SBD calculations. - + """Helper class to configure CPU vs GPU execution for SBD calculations. + Usage:: from sbd.device_config import DeviceConfig @@ -36,13 +34,12 @@ class DeviceConfig: results = solve_sci_batch(..., device_config=device) """ - + def __init__(self, device: str = 'cpu', use_precalculated_dets: bool = True, max_memory_gb: int = -1, use_gpu: bool | None = None): - """ - Initialize device configuration. + """Initialize device configuration. Args: device: Backend device key — 'cpu', 'gpu' (NVHPC Thrust, @@ -62,11 +59,10 @@ def __init__(self, device: str = 'cpu', self.use_gpu = device != 'cpu' self.use_precalculated_dets = use_precalculated_dets self.max_memory_gb = max_memory_gb - + @classmethod def auto(cls, max_memory_gb: int = -1) -> 'DeviceConfig': - """ - Auto-detect the best available backend and use it. + """Auto-detect the best available backend and use it. Chooses among the backends that were actually built, not merely the hardware present, so an AMD host or a CPU-only build is never handed a @@ -115,7 +111,7 @@ def cpu(cls) -> 'DeviceConfig': @classmethod def gpu(cls, use_precalculated_dets: bool = True, max_memory_gb: int = -1) -> 'DeviceConfig': - """Force NVHPC Thrust GPU execution. **NVIDIA only.** + """Force NVHPC Thrust GPU execution (**NVIDIA only**). Requires SBD compiled with THRUST (the ``_core_gpu_thrust`` extension, i.e. ``SBD_BUILD_BACKEND=gpu``, or the default ``auto``). @@ -169,7 +165,7 @@ def gpu_nvidia_omp(cls, max_memory_gb: int = -1) -> 'DeviceConfig': DeprecationWarning, stacklevel=2, ) return cls.gpu_omp(max_memory_gb=max_memory_gb) - + # Cached detection results (None = not yet checked) _cuda_cache: bool | None = None _hip_cache: bool | None = None @@ -216,10 +212,9 @@ def _check_hip(cls) -> bool: except Exception: cls._hip_cache = False return cls._hip_cache - + def apply(self, sbd_config) -> None: - """ - Apply device configuration to an SBD TPB_SBD configuration object. + """Apply device configuration to an SBD TPB_SBD configuration object. Args: sbd_config: sbd.TPB_SBD configuration object @@ -238,8 +233,9 @@ def apply(self, sbd_config) -> None: "backend module — likely a CPU/OMP-offload build " "loaded under device='gpu'. Numerics still run on " "the selected backend.") - + def __repr__(self) -> str: + """Return a readable representation of the configuration.""" if self.device == 'cpu': return "DeviceConfig(CPU)" return (f"DeviceConfig(device={self.device!r}, " @@ -248,9 +244,8 @@ def __repr__(self) -> str: def get_device_info() -> dict: - """ - Get information about available compute devices. - + """Get information about available compute devices. + Returns: Dictionary with device information """ @@ -261,7 +256,7 @@ def get_device_info() -> dict: 'gpu_type': None, 'gpu_count': 0 } - + if info['cuda_available']: info['gpu_available'] = True info['gpu_type'] = 'CUDA' @@ -271,7 +266,7 @@ def get_device_info() -> dict: capture_output=True, text=True, timeout=2, ) if result.returncode == 0: - info['gpu_count'] = len([l for l in result.stdout.split('\n') if l.strip()]) + info['gpu_count'] = len([ln for ln in result.stdout.split('\n') if ln.strip()]) except Exception: pass @@ -292,7 +287,7 @@ def get_device_info() -> dict: info['gpu_count'] = len(set(ids)) except Exception: pass - + return info diff --git a/python/sbd_solver.py b/python/sbd_solver.py index 59f975f..08e4827 100644 --- a/python/sbd_solver.py +++ b/python/sbd_solver.py @@ -10,8 +10,7 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. -""" -SBD solver wrapper compatible with qiskit-addon-sqd interface. +"""SBD solver wrapper compatible with qiskit-addon-sqd interface. This module provides functions that wrap the SBD (Selected Basis Diagonalization) library to be compatible with the qiskit-addon-sqd diagonalize_fermionic_hamiltonian @@ -20,11 +19,10 @@ from __future__ import annotations -import tempfile import shutil +import tempfile +from collections.abc import Callable from pathlib import Path -from collections.abc import Sequence -from typing import Callable import numpy as np from mpi4py import MPI @@ -80,8 +78,7 @@ def solve_sci( device_config=None, fcidump_path: str | Path | None = None, ) -> SCIResult: - """ - Diagonalize Hamiltonian in subspace defined by CI strings using SBD. + """Diagonalize Hamiltonian in subspace defined by CI strings using SBD. Args: ci_strings: Pair (strings_a, strings_b) of CI string arrays. @@ -158,8 +155,7 @@ def _solve_sci_core( device_config=None, ecore_offset: float = 0.0, ) -> SCIResult: - """ - Inner diagonalization kernel that operates on a pre-loaded FCIDUMP object. + """Inner diagonalization kernel that operates on a pre-loaded FCIDUMP object. Separated from solve_sci so that solve_sci_batch can write and load the FCIDUMP only once and reuse it across all batches. @@ -522,8 +518,7 @@ def solve_sci_batch( device_config=None, fcidump_path: str | Path | None = None, ) -> list[SCIResult]: - """ - Diagonalize Hamiltonian in multiple subspaces using SBD. + """Diagonalize Hamiltonian in multiple subspaces using SBD. The FCIDUMP file is loaded once and reused across all batches. @@ -784,9 +779,9 @@ def create_sbd_solver( clean_temp_dir: bool = True, device_config=None, ) -> Callable: - """ - Create a configured SBD solver function for use with - diagonalize_fermionic_hamiltonian. + """Create a configured SBD solver function. + + The result is suitable for use with ``diagonalize_fermionic_hamiltonian``. Example: >>> from functools import partial diff --git a/releasenotes/notes/enable-linters-77600380da49ab67.yaml b/releasenotes/notes/enable-linters-77600380da49ab67.yaml new file mode 100644 index 0000000..741456d --- /dev/null +++ b/releasenotes/notes/enable-linters-77600380da49ab67.yaml @@ -0,0 +1,16 @@ +--- +upgrade: + - | + The repository now runs ``ruff check``, a copyright-header check + (``ruff check --preview --select CPY001``) and ``reno lint`` in CI, through + new ``style`` and ``lint`` tox environments. Run ``tox -e style`` to apply + the automatic fixes and ``tox -e lint`` for a non-destructive check. + + These environments set ``skip_install``, so neither compiles the extension + and neither needs MPI or BLAS present. +fixes: + - | + :func:`sbd.init` now chains the underlying ``ImportError`` when ``mpi4py`` + is missing, so the original traceback is no longer discarded. Backend + loading does the same for the error that prevented an extension from + importing. diff --git a/test/test_gdb_drivers.py b/test/test_gdb_drivers.py index 4f49a2c..7b124b4 100644 --- a/test/test_gdb_drivers.py +++ b/test/test_gdb_drivers.py @@ -33,6 +33,7 @@ from __future__ import annotations +import itertools import json import pathlib import re @@ -228,7 +229,7 @@ def test_heatbath_ladder_grows_and_lowers_the_energy(tmp_path): energies = [entry["energy"] for entry in rounds] assert dims == sorted(dims), f"subspace shrank across rounds: {dims}" assert dims[-1] > dims[0], f"expansion added nothing: {dims}" - for before, after in zip(energies, energies[1:]): + for before, after in itertools.pairwise(energies): assert after <= before + 1e-10, f"energy rose across a round: {energies}" diff --git a/test/test_gdb_equivalence.py b/test/test_gdb_equivalence.py index bfdcda5..279630d 100644 --- a/test/test_gdb_equivalence.py +++ b/test/test_gdb_equivalence.py @@ -334,7 +334,7 @@ def test_gdb_rejects_a_task_split(backend, h2o): @pytest.fixture(scope="module") def h2o_product(backend, h2o): """The truncated product basis, packed and globally sorted.""" - fcidump, norb, _, alpha = h2o + _, norb, _, alpha = h2o strings = [_interleave(a, b) for a, b in itertools.product(alpha, alpha)] packed = sbd.sort_bitarray_array( sbd.from_strings(strings, BIT_LENGTH, 2 * norb) @@ -350,7 +350,7 @@ def test_from_strings_matches_the_per_determinant_form(backend, h2o): determinant, which issue #31 measures as a real cost at tens of thousands of strings. It is only worth having if it is the same function, hence this. """ - fcidump, norb, _, alpha = h2o + _, norb, _, alpha = h2o strings = [_interleave(a, b) for a, b in itertools.product(alpha[:6], alpha[:6])] bulk = sbd.from_strings(strings, BIT_LENGTH, 2 * norb) one_by_one = [backend.from_string(s, BIT_LENGTH, 2 * norb) for s in strings] @@ -661,7 +661,7 @@ def test_gdb_fe4s4_at_realistic_size(): assert completed.returncode == 0, ( f"subprocess failed:\n{completed.stdout[-2000:]}\n{completed.stderr[-2000:]}" ) - line = [l for l in completed.stdout.splitlines() if l.startswith("ENERGY")] + line = [ln for ln in completed.stdout.splitlines() if ln.startswith("ENERGY")] assert line, f"no energy reported:\n{completed.stdout[-2000:]}" _, energy, _, dim = line[-1].split() assert int(dim) == 59536 diff --git a/tox.ini b/tox.ini index 344d95a..b7e6c85 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -minversion = 4.4.4 +minversion = 4.28.0 envlist = py{311,312,313,314}{,-notebook}, mpi, docs isolated_build = True @@ -93,6 +93,32 @@ extras = commands = pytest --nbmake --nbmake-timeout=3000 {posargs} examples/ +[testenv:style] +# Linting reads the source; it does not import the package, so there is no reason to +# compile the extension for it. `skip_install` keeps `tox -e style` fast and lets it +# run on a machine with no MPI or BLAS at all. +skip_install = true +dependency_groups = + style +allowlist_externals = + ruff +commands = + ruff check --fix python/ test/ docs/ examples/ + +[testenv:lint] +skip_install = true +dependency_groups = + lint +allowlist_externals = + reno + ruff +commands = + ruff check python/ test/ docs/ examples/ + # The copyright-header check is a separate invocation: it needs --preview, and the + # notebook is excluded because the header belongs in the .py sources only. + ruff check --preview --select CPY001 --exclude "*.ipynb" python/ test/ docs/ examples/ + reno lint + [testenv:docs] # Unlike a pure-Python project, the docs build cannot skip installing the package: # autodoc imports `sbd` to read its docstrings, and conf.py reads the version from the From e22b465ec5fa2fd5796daebcd9a26707d6e1f20f Mon Sep 17 00:00:00 2001 From: Jim Garrison Date: Wed, 21 Oct 2026 22:44:10 -0400 Subject: [PATCH 2/3] Stop reordering imports in the examples The isort pass reached further into the examples than it should have. `known-first-party = ["sbd"]` makes `sbd` its own import group, and isort separates groups with a blank line. In `python/` and `test/` that is the point: without it `import sbd` groups with the third-party imports. But in a four-line function-local block it splits `from mpi4py import MPI` from the `import sbd` beneath it for no gain, which is what #68 was reordering in run_gdb_heatbath.py's main(). Ignore `I001` for `examples/` and say why on the isort block. The examples are the one place in this repo where reading order was chosen by hand, so they keep it. Restoring those files also drops reformatting the final config never asked for. `D` is already ignored for `examples/`, and `E501` is ignored globally, so the docstring summary moves and the wrapped qiskit_addon_sqd import were both churn from a `tox -e style` run made before those ignores existed. What remains in the examples is trailing whitespace and, in the notebook, splitting `import json, numpy as np` and dropping an `f` prefix with no placeholders. The release note moves from `upgrade` to `other`. Nothing here asks anything of a user of the package. Verified with `tox -e py314` (51 passed, 35 skipped), `tox -e lint` and `tox -e docs`. Assisted-by: Claude Opus 5 --- examples/gdb/run_gdb_diag.py | 3 ++- examples/gdb/run_gdb_heatbath.py | 6 +++--- examples/tpb/run_sbd_diag.py | 4 ++-- examples/tpb/run_sqd_enlarge_subspace_sbd.py | 7 ++----- examples/tpb/run_sqd_sbd.ipynb | 9 ++++----- examples/tpb/run_sqd_sbd.py | 2 +- pyproject.toml | 6 ++++++ releasenotes/notes/enable-linters-77600380da49ab67.yaml | 2 +- 8 files changed, 21 insertions(+), 18 deletions(-) diff --git a/examples/gdb/run_gdb_diag.py b/examples/gdb/run_gdb_diag.py index ea02cb8..8c72b30 100755 --- a/examples/gdb/run_gdb_diag.py +++ b/examples/gdb/run_gdb_diag.py @@ -12,7 +12,8 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. -"""Standalone GDB diagonalization over an explicit determinant list, in memory. +""" +Standalone GDB diagonalization over an explicit determinant list, in memory. GDB (general determinant basis) spans the subspace with the determinants it is given. TPB spans it with the Cartesian product of an alpha and a beta list, so diff --git a/examples/gdb/run_gdb_heatbath.py b/examples/gdb/run_gdb_heatbath.py index 9063a67..2f9b744 100755 --- a/examples/gdb/run_gdb_heatbath.py +++ b/examples/gdb/run_gdb_heatbath.py @@ -12,7 +12,8 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. -"""Grow a GDB subspace with SBD's own heatbath expansion, round after round. +""" +Grow a GDB subspace with SBD's own heatbath expansion, round after round. This is selected CI (HCI) driven from Python: diagonalize, let SBD expand the subspace from the resulting wavefunction, diagonalize the larger subspace, repeat. @@ -402,9 +403,8 @@ def reshard(det, args, comm, rank, size): def main(): args = parse_args() - from mpi4py import MPI - import sbd + from mpi4py import MPI sbd.init(device=args.device) backend = sbd.get_backend(args.device) diff --git a/examples/tpb/run_sbd_diag.py b/examples/tpb/run_sbd_diag.py index 59efcff..352ec9d 100644 --- a/examples/tpb/run_sbd_diag.py +++ b/examples/tpb/run_sbd_diag.py @@ -12,7 +12,8 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. -"""Standalone SBD diagonalization (no SQD loop, no qiskit dependency). +""" +Standalone SBD diagonalization (no SQD loop, no qiskit dependency). Runs a single TPB diagonalization from an FCIDUMP file and alpha determinant file using the SBD library directly. @@ -49,7 +50,6 @@ import numpy as np - def parse_args(): """Parse command line arguments for all TPB_SBD parameters""" parser = argparse.ArgumentParser( diff --git a/examples/tpb/run_sqd_enlarge_subspace_sbd.py b/examples/tpb/run_sqd_enlarge_subspace_sbd.py index 4c0dbdf..f767cba 100644 --- a/examples/tpb/run_sqd_enlarge_subspace_sbd.py +++ b/examples/tpb/run_sqd_enlarge_subspace_sbd.py @@ -59,10 +59,7 @@ from mpi4py import MPI from pyscf import ao2mo, tools from qiskit.primitives import BitArray -from qiskit_addon_sqd.fermion import ( - diagonalize_fermionic_hamiltonian, - enlarge_batch_from_transitions, -) +from qiskit_addon_sqd.fermion import diagonalize_fermionic_hamiltonian, enlarge_batch_from_transitions def parse_args(): @@ -321,8 +318,8 @@ def main(): print(f"Device: {args.device}") print() - from sbd.device_config import DeviceConfig from sbd.sbd_solver import solve_sci_batch + from sbd.device_config import DeviceConfig device_str = args.device if device_str == "auto": diff --git a/examples/tpb/run_sqd_sbd.ipynb b/examples/tpb/run_sqd_sbd.ipynb index 868c90b..90dbe63 100644 --- a/examples/tpb/run_sqd_sbd.ipynb +++ b/examples/tpb/run_sqd_sbd.ipynb @@ -57,19 +57,18 @@ } ], "source": [ - "import json\n", "from functools import partial\n", - "from pathlib import Path\n", - "\n", + "import json\n", "import numpy as np\n", + "from pathlib import Path\n", "from mpi4py import MPI\n", "from pyscf import ao2mo, tools\n", - "from qiskit.primitives import BitArray\n", "from qiskit_addon_sqd.fermion import diagonalize_fermionic_hamiltonian\n", + "from qiskit.primitives import BitArray\n", "\n", "import sbd\n", - "from sbd.device_config import DeviceConfig, print_device_info\n", "from sbd.sbd_solver import solve_sci_batch\n", + "from sbd.device_config import DeviceConfig, print_device_info\n", "\n", "comm = MPI.COMM_WORLD\n", "print(f'SBD version : {sbd.__version__}')\n", diff --git a/examples/tpb/run_sqd_sbd.py b/examples/tpb/run_sqd_sbd.py index 50b2f61..4b37132 100644 --- a/examples/tpb/run_sqd_sbd.py +++ b/examples/tpb/run_sqd_sbd.py @@ -385,8 +385,8 @@ def main(): # solve_sci_batch auto-initializes the SBD backend on first call and # falls back to MPI.COMM_WORLD when mpi_comm is not provided, so no # explicit sbd.init() / mpi_comm threading is needed in user code. - from sbd.device_config import DeviceConfig, print_device_info from sbd.sbd_solver import solve_sci_batch + from sbd.device_config import DeviceConfig, print_device_info device_str = args.device if device_str == "auto": diff --git a/pyproject.toml b/pyproject.toml index f405e4f..2861836 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -124,6 +124,7 @@ ignore = [ # surrounding discussion. "examples/**/*" = [ "D", # pydocstyle + "I001", # unsorted-imports: see the isort note below "RUF001", # ambiguous unicode character in string "RUF003", # ambiguous unicode character in comment "SIM102", # collapsible if: the nesting mirrors the explanation @@ -142,6 +143,11 @@ ignore = [ # The package directory is `python/` but it imports as `sbd` (see # `[tool.setuptools] package-dir`), so isort cannot infer that `sbd` is ours and # would otherwise group it with the third-party imports. +# +# This makes `sbd` its own import group, which isort separates with a blank line. +# That is what we want in `python/` and `test/`, but in the examples it splits +# short function-local blocks like `from mpi4py import MPI` next to `import sbd` +# for no gain, so `I001` is ignored for `examples/` above. known-first-party = ["sbd"] [tool.ruff.lint.pydocstyle] diff --git a/releasenotes/notes/enable-linters-77600380da49ab67.yaml b/releasenotes/notes/enable-linters-77600380da49ab67.yaml index 741456d..7c2bb91 100644 --- a/releasenotes/notes/enable-linters-77600380da49ab67.yaml +++ b/releasenotes/notes/enable-linters-77600380da49ab67.yaml @@ -1,5 +1,5 @@ --- -upgrade: +other: - | The repository now runs ``ruff check``, a copyright-header check (``ruff check --preview --select CPY001``) and ``reno lint`` in CI, through From 98873e94d9eb3ec9a423ddaad6331c057de5327f Mon Sep 17 00:00:00 2001 From: Jim Garrison Date: Fri, 23 Oct 2026 13:47:55 -0400 Subject: [PATCH 3/3] Apply suggestion from @garrison --- releasenotes/notes/enable-linters-77600380da49ab67.yaml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/releasenotes/notes/enable-linters-77600380da49ab67.yaml b/releasenotes/notes/enable-linters-77600380da49ab67.yaml index 7c2bb91..45806d1 100644 --- a/releasenotes/notes/enable-linters-77600380da49ab67.yaml +++ b/releasenotes/notes/enable-linters-77600380da49ab67.yaml @@ -8,9 +8,3 @@ other: These environments set ``skip_install``, so neither compiles the extension and neither needs MPI or BLAS present. -fixes: - - | - :func:`sbd.init` now chains the underlying ``ImportError`` when ``mpi4py`` - is missing, so the original traceback is no longer discarded. Backend - loading does the same for the error that prevented an extension from - importing.