Add native cube generator backed by GauXC OrbitalEvaluator - #461
Merged
Conrad Johnston (ConradJohnston) merged 36 commits intoAug 19, 2026
Conversation
📊 Coverage Summary
Detailed Coverage ReportsC++ Coverage DetailsPython Coverage DetailsPybind11 Coverage Details |
Conrad Johnston (ConradJohnston)
force-pushed
the
feature/cojoh/native-cubegen-v2
branch
from
April 29, 2026 22:00
e541553 to
2854b41
Compare
Conrad Johnston (ConradJohnston)
changed the title
Conrad Johnston (ConradJohnston)
force-pushed
the
feature/cojoh/native-cubegen-v2
branch
3 times, most recently
from
April 29, 2026 22:31
fb6328d to
6ac7498
Compare
Conrad Johnston (ConradJohnston)
force-pushed
the
feature/cojoh/native-cubegen-v2
branch
from
August 10, 2026 16:52
6ac7498 to
099d514
Compare
Replace PySCF cubegen with a native C++ implementation that uses GauXC's OrbitalEvaluator for MO and density evaluation on cube grids, and GauXC's write_cube for file output. This eliminates: - The vendored gauxc-private/ header (no longer needed — OrbitalEvaluator is the public API for point-set evaluation) - 600+ lines of hand-rolled collocation batching, GEMM, and text formatting code (all now in GauXC upstream) GauXC pin bumped to ConradJohnston/GauXC@10f59a8 which includes OrbitalEvaluator with shell screening, pipelined collocation+GEMM, CubeGrid-native overloads, and parallel cube writer.
Conrad Johnston (ConradJohnston)
force-pushed
the
feature/cojoh/native-cubegen-v2
branch
from
August 10, 2026 22:54
7e0c287 to
b51aefd
Compare
Conrad Johnston (ConradJohnston)
marked this pull request as ready for review
August 11, 2026 18:11
Conrad Johnston (ConradJohnston)
requested a review
from Nathan Baker (sobolevnrm)
as a code owner
August 11, 2026 18:11
Copilot started reviewing on behalf of
Conrad Johnston (ConradJohnston)
August 11, 2026 18:11
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a native C++ Gaussian cube-field generator backed by GauXC’s OrbitalEvaluator, updates the GauXC dependency to the specified upstream commit, and adds focused regression tests plus a clang-cl compatibility patch for Gau2Grid.
Changes:
- Added
CubeGrid/CubeGeneratorto evaluate orbitals and density on 3D grids and optionally write cube files viaGauXC::write_cube. - Updated GauXC pin to commit
162e4562…and added a clang-cl patch hook for Gau2Grid’s generated helper. - Added C++ regression tests for grid construction, orbital/density evaluation, and input validation; added a vcpkg overlay port for
libaec.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
cpp/include/qdk/chemistry/cube/cube_generator.hpp |
Declares CubeGrid, CubeGenerator, and generate_orbital_cubes API. |
cpp/src/qdk/chemistry/cube/cube_generator.cpp |
Implements GauXC-backed orbital/density evaluation and cube writing. |
cpp/src/qdk/chemistry/cube/CMakeLists.txt |
Adds cube generator sources to the chemistry target. |
cpp/CMakeLists.txt |
Wires the new cube subdirectory into the C++ build. |
cpp/tests/test_cube_generator.cpp |
Adds regression tests for grid sizing, evaluation correctness, and error cases. |
cpp/cmake/third_party.cmake |
Updates GauXC commit pin and applies a clang-cl patch during FetchContent. |
cpp/cmake/patches/gauxc-clang-cl-gau2grid-stdlib.cmake |
Implements Gau2Grid helper patching for clang-cl builds. |
.pipelines/install-scripts/install-cpp-deps-windows.ps1 |
Applies the same GauXC clang-cl patch in the Windows dependency install script. |
cpp/manifest/qdk-chemistry/cgmanifest.json |
Updates the recorded GauXC commit hash to the new pin. |
vcpkg-overlay/ports/libaec/vcpkg.json |
Adds an overlay port definition for libaec. |
vcpkg-overlay/ports/libaec/portfile.cmake |
Builds/installs libaec and fixes up exported targets for vcpkg usage. |
vcpkg-overlay/ports/libaec/usage |
Documents CMake usage for the overlay libaec port. |
Suppressed comments (1)
cpp/tests/test_cube_generator.cpp:110
- This test uses
std::numbers::pi(C++20). Since the tests are built as C++17, compute π viastd::acos(-1.0)instead.
const double r_squared = grid.origin.squaredNorm();
const double normalization = 2.0 * std::pow(2.0 / std::numbers::pi, 0.75);
const double radial = normalization * std::exp(-r_squared);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot started reviewing on behalf of
Conrad Johnston (ConradJohnston)
August 11, 2026 19:55
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (3)
cpp/cmake/patches/gauxc-clang-cl-gau2grid-stdlib.cmake:10
- This patch script assumes the helper file always exists and only checks for a very specific include ordering. If GauXC changes the path or if <stdlib.h> is already included elsewhere, the script can fail the configure step (missing file) or inject a duplicate include.
set(_helper "external/gau2grid/generated_source/gau2grid_helper.c")
file(READ "${_helper}" _content)
if(NOT _content MATCHES "#include <stdlib.h>\n#include <math.h>")
string(REPLACE "#include <math.h>"
"#include <stdlib.h>\n#include <math.h>"
_content "${_content}")
cpp/src/qdk/chemistry/utils/cube_generator.cpp:57
- Error message capitalization is inconsistent ("gauXC") and likely meant to reference GauXC. This makes logs harder to search and is inconsistent with other GauXC references in this PR.
constexpr auto max =
static_cast<std::size_t>(std::numeric_limits<int64_t>::max());
if (nx > max / ny || nx * ny > max / nz)
throw std::overflow_error("CubeGrid: point count exceeds gauXC's limit.");
return nx * ny * nz;
cpp/src/qdk/chemistry/utils/cube_generator.cpp:186
- generate_orbital_cubes calls std::filesystem::create_directories(output_dir) without validating output_dir. Passing an empty string can throw a filesystem_error (platform-dependent) instead of a clear invalid_argument, which is a surprising API contract for a utility function.
CubeGenerator gen(wfn.get_orbitals()->get_basis_set());
const auto& C_a = wfn.get_orbitals()->coefficients()->block(
{data::axes::alpha(), data::axes::alpha()});
std::filesystem::create_directories(output_dir);
Validate grid dimensions and unsupported basis shells, migrate orbital access to the current API, and add focused cube generator regression tests.
Keep preinstalled CI dependencies aligned with the CMake pin so the upstream cube-generation headers are available on every platform.
Avoid the unnecessarily strict 1e-12 shell tolerance so cube evaluation benefits from gauXC's performance-oriented default while retaining conservative accuracy.
Override the vcpkg libaec source archive with its canonical GitHub mirror to avoid repeated GitLab rate-limit failures in Windows dependency installation.
Include stdlib.h in GauXC's generated Gau2Grid helper when preparing clang-cl dependencies so exit() is declared under C99 rules.
The class docstring listed origin and spacing under Attributes: while both were also exposed as read/write properties. With undoc-members enabled, Sphinx documented each of them twice and the autosummary build fails on any warning. Move the descriptions onto the properties themselves.
Copilot started reviewing on behalf of
Conrad Johnston (ConradJohnston)
August 17, 2026 14:53
View session
Copilot stopped reviewing on behalf of
Conrad Johnston (ConradJohnston) due to an error
August 17, 2026 14:53
The bounds check ran on the already-narrowed value, so a shell with more than INT32_MAX primitives would truncate to a small count and slip past it, building a silently wrong shell. Compare the original size instead.
Copilot started reviewing on behalf of
Conrad Johnston (ConradJohnston)
August 17, 2026 16:39
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (5)
python/src/pybind11/utils/cube_generator.cpp:204
- The
generate_orbital_cubesPython docstring saysoutput_dirmust already exist, but the C++ implementation creates it (std::filesystem::create_directories). This mismatch can mislead users; update the docstring to match the actual behavior.
Args:
orbitals: Orbitals supplying the basis set and coefficients.
indices: Zero-based orbital indices to write.
output_dir: Existing directory to write the cube files into.
grid: Grid to evaluate on.
label_prefix: Prefix of the generated file names.
python/src/qdk_chemistry/utils/cubegen.py:68
- The docstring documents only ValueError for invalid
backend, but thebackend="pyscf"branch importspyscflazily and will raise ImportError when PySCF isn't installed (e.g., Windows). Document this explicitly so callers know what exception to expect.
Raises:
ValueError: If ``backend`` is not ``"native"`` or ``"pyscf"``.
cpp/src/qdk/chemistry/utils/cube_generator.cpp:277
- For restricted orbitals, the generated filename has no spin suffix, but the cube comment line currently labels the orbital as "(alpha)". That’s misleading for restricted wavefunctions (there is only a spatial orbital). Consider dropping the spin qualifier in this branch.
// Restricted: a single spatial cube with no spin suffix. Unrestricted:
// separate alpha (`_a`) and beta (`_b`) cubes, mirroring `cubegen.py`.
if (restricted) {
emit(C_a.col(p), stem + ".cube",
"Orbital " + std::to_string(p) + " (alpha)");
python/tests/test_utils_cube_generator.py:80
- Most tests in this file (including all
CubeGeneratorandgenerate_orbital_cubescoverage) are skipped when PySCF is unavailable. Since one of the main goals of the native backend is to enable Windows usage without PySCF, this leaves the Python bindings largely untested on that target. Consider adding at least a small PySCF-free smoke test (e.g., build aBasisSetviaBasisSet.from_basis_name, create identityOrbitals, and assertCubeGenerator.orbital/generate_orbital_cubesrun and write files) and only skipping the strict PySCF equivalence tests.
@pytest.mark.skipif(not PYSCF_AVAILABLE, reason="PySCF not available")
class TestCubeGenerator:
"""Tests for orbital and density evaluation."""
python/tests/test_utils_cubegen.py:126
- This module is globally skipped when PySCF is not available (
pytestmark = skipif(...)), but the new default backend isnativespecifically to work without PySCF (notably on Windows). As-is, there’s no test that exercisesgenerate_cubefiles_from_orbitals(..., backend="native")in an environment where PySCF truly cannot be imported. Consider moving the global skip down onto only the PySCF-dependent tests and adding a small smoke test that asserts importingqdk_chemistry.utils.cubegenand calling the native backend does not import PySCF (e.g., via an import hook).
def test_native_backend_matches_pyscf_backend(self):
"""Both backends must place the grid identically and agree on every field value.
This is the guard on making the native backend the default: switching
it must not move a single grid point or change a single value beyond
the precision the cube format itself can represent.
"""
orbitals = _o2_orbitals()
kwargs = {"indices": [0, 3], "grid_size": (8, 8, 8)}
native = generate_cubefiles_from_orbitals(orbitals, backend="native", **kwargs)
reference = generate_cubefiles_from_orbitals(orbitals, backend="pyscf", **kwargs)
The single hydrogen atom at the origin is already provided by testing::create_hydrogen_structure in ut_common.hpp, and EvaluatesHydrogenOrbitalAndDensity was rebuilding the grid that single_point_grid already returns.
Copilot started reviewing on behalf of
Conrad Johnston (ConradJohnston)
August 17, 2026 17:28
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (1)
python/src/pybind11/utils/cube_generator.cpp:204
- The
generate_orbital_cubesPython docstring saysoutput_dirmust be an existing directory, but the C++ implementation explicitly callsstd::filesystem::create_directories(output_dir)(so it will create it). This is user-facing API documentation and should match actual behavior.
orbitals: Orbitals supplying the basis set and coefficients.
indices: Zero-based orbital indices to write.
output_dir: Existing directory to write the cube files into.
grid: Grid to evaluate on.
label_prefix: Prefix of the generated file names.
Agam Shayit (agamshayit-ms)
requested changes
Aug 18, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 243329d9-adbf-472d-ad3d-136741e7e8cf
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (2)
python/tests/test_utils_cube_generator.py:105
TestCubeGenerator(which contains the main behavioral coverage forCubeGenerator) is entirely skipped when PySCF isn't installed. That leaves the new native cube backend effectively untested on Windows (the primary motivation for this PR), even thoughCubeGeneratoritself doesn't depend on PySCF.
Add at least one smoke test that constructs a minimal spherical BasisSet directly (no PySCF) and verifies CubeGenerator.orbital(...) returns the expected shape and can write a cube file.
@pytest.mark.skipif(not PYSCF_AVAILABLE, reason="PySCF not available")
class TestCubeGenerator:
"""Tests for orbital and density evaluation."""
cpp/cmake/modules/DependencyManager.cmake:88
REQUIRED_HEADERvalidation usescheck_include_file_cxx, but only setsCMAKE_REQUIRED_LIBRARIESto${ARG_INSTALL_TARGET}. That does not reliably propagate the target's include directories into the check, so this can produce false negatives (especially when dependency fetching is disabled and the dependency is only available viafind_package).
Populate CMAKE_REQUIRED_INCLUDES from the discovered target's INTERFACE_INCLUDE_DIRECTORIES (when the install target is a CMake target) before calling check_include_file_cxx.
set(_saved_cmake_required_libraries "${CMAKE_REQUIRED_LIBRARIES}")
set(CMAKE_REQUIRED_LIBRARIES "${ARG_INSTALL_TARGET}")
check_include_file_cxx("${ARG_REQUIRED_HEADER}"
${_required_header_check})
set(CMAKE_REQUIRED_LIBRARIES "${_saved_cmake_required_libraries}")
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (3)
python/src/qdk_chemistry/utils/cubegen.py:71
- The docstring notes that the PySCF backend raises ImportError when PySCF isn’t installed, but the formal
Raises:section only lists ValueError. This makes the documented exception contract incomplete for callers doing exception handling.
Raises:
ValueError: If ``backend`` is not ``"native"`` or ``"pyscf"``, or if
``backend="pyscf"`` is requested for a Cartesian basis.
python/src/qdk_chemistry/utils/cubegen.py:122
- For the native backend, cube files are written with an empty comment line (no
comment=is passed toCubeGenerator.orbital). That makes returned cube text / written cubes less self-describing than the PySCF backend and thangenerate_orbital_cubes(which embeds an orbital label in the comment). Consider passing a stable label (e.g. derived from the output filename) so the cube header identifies what was generated.
else:
outfile_name = output_folder / label
_write_cube(coeff, outfile_name)
python/tests/test_utils_cube_generator.py:102
- Most of the Python binding tests for
CubeGenerator/generate_orbital_cubesare skipped when PySCF isn’t installed. Since this PR’s key goal is enabling cube generation on Windows (where PySCF is typically unavailable), it would be good to add at least one smoke test that exercises the native backend end-to-end without requiring PySCF (construct a minimalBasisSet/Orbitals, write a cube intotmp_path, and assert it exists).
@pytest.mark.skipif(not PYSCF_AVAILABLE, reason="PySCF not available")
class TestCubeGenerator:
"""Tests for orbital and density evaluation."""
Agam Shayit (agamshayit-ms)
requested changes
Aug 18, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 243329d9-adbf-472d-ad3d-136741e7e8cf
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
python/tests/test_utils_cube_generator.py:99
- The native backend is meant to enable cube generation when PySCF is not installed (notably on Windows), but this test module only exercises native evaluation under
PYSCF_AVAILABLEskips. Adding a small, non-PySCF test that callsgenerate_cubefiles_from_orbitals(..., backend="native")would ensure the new default path is exercised on platforms without PySCF and that the bindings work end-to-end.
def test_pyscf_backend_rejects_cartesian_basis_before_import(tmp_path):
"""PySCF cannot consume the Cartesian AO coefficient layout."""
structure = Structure(["O"], np.zeros((1, 3)))
shell = Shell(0, OrbitalType.D, np.array([1.0]), np.array([1.0]))
basis_set = BasisSet("cartesian-d", [shell], structure, AOType.Cartesian)
nbf = basis_set.get_num_atomic_orbitals()
orbitals = Orbitals(np.eye(nbf), None, None, basis_set)
with pytest.raises(ValueError, match="does not support Cartesian"):
generate_cubefiles_from_orbitals(
orbitals,
output_folder=tmp_path,
indices=[0],
grid_size=(2, 2, 2),
backend="pyscf",
)
assert list(tmp_path.iterdir()) == []
cpp/src/qdk/chemistry/utils/cube_generator.cpp:32
CubeGrid::from_basis_setrejects negative margins but will accept NaN/Inf margins (sincemargin < 0.0is false for NaN). That can propagate NaNs intoorigin/spacingand then into GauXC evaluation/writing. Since this is a public API (also bound to Python), it should reject non-finite margins explicitly.
if (nx == 0 || ny == 0 || nz == 0)
throw std::invalid_argument("CubeGrid: dimensions must be positive.");
if (margin < 0.0)
throw std::invalid_argument("CubeGrid: margin cannot be negative.");
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 243329d9-adbf-472d-ad3d-136741e7e8cf
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
python/src/pybind11/utils/cube_generator.cpp:203
- The
generate_orbital_cubesbinding docstring saysoutput_dirmust be an existing directory, but the C++ implementation callsstd::filesystem::create_directories(output_dir)and will create it if missing. This mismatch can mislead callers and test expectations.
Args:
orbitals: Orbitals supplying the basis set and coefficients.
indices: Zero-based orbital indices to write.
output_dir: Existing directory to write the cube files into.
grid: Grid to evaluate on.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 243329d9-adbf-472d-ad3d-136741e7e8cf
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (2)
python/src/qdk_chemistry/utils/cubegen.py:70
- The docstring says the PySCF backend raises ImportError when PySCF is unavailable, but the "Raises:" section currently documents only ValueError. This makes the API contract incomplete for users relying on the raised-exceptions list.
Raises:
ValueError: If ``backend`` is not ``"native"`` or ``"pyscf"``, or if
``backend="pyscf"`` is requested for a Cartesian basis.
python/tests/test_utils_cube_generator.py:99
- This test module has a non-skipped test that exercises only the PySCF backend's early rejection path. Since the PR's main goal is to make cube generation work without PySCF (e.g., on Windows), it would be valuable to add at least one unskipped test that calls generate_cubefiles_from_orbitals with backend="native" and asserts it writes a valid cube (or returns cube text) without requiring PySCF.
def test_pyscf_backend_rejects_cartesian_basis_before_import(tmp_path):
"""PySCF cannot consume the Cartesian AO coefficient layout."""
structure = Structure(["O"], np.zeros((1, 3)))
shell = Shell(0, OrbitalType.D, np.array([1.0]), np.array([1.0]))
basis_set = BasisSet("cartesian-d", [shell], structure, AOType.Cartesian)
nbf = basis_set.get_num_atomic_orbitals()
orbitals = Orbitals(np.eye(nbf), None, None, basis_set)
with pytest.raises(ValueError, match="does not support Cartesian"):
generate_cubefiles_from_orbitals(
orbitals,
output_folder=tmp_path,
indices=[0],
grid_size=(2, 2, 2),
backend="pyscf",
)
assert list(tmp_path.iterdir()) == []
Agam Shayit (agamshayit-ms)
approved these changes
Aug 18, 2026
This was referenced Aug 21, 2026
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
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.
Why
generate_cubefiles_from_orbitalswent through PySCF. PySCF is pinnedsys_platform != 'win32'inpython/pyproject.toml, andutils/cubegen.pyimported
pyscf.tools.cubegenat module scope, so the module was unimportableon Windows and cube generation was simply unavailable there. It also meant
orbital visualization pulled in a third-party quantum chemistry package for
something the library can evaluate itself.
How
Adds a native
CubeGeneratorinqdk::chemistry::utilsthat evaluates orbitalsand densities on a
CubeGridusing GauXC'sOrbitalEvaluator, writes Gaussiancube files through
GauXC::write_cube, and is exposed to Python. It is now thedefault backend for
generate_cubefiles_from_orbitals, which gives Windows cubegeneration for the first time.
backend="pyscf"still works wherever PySCF isinstalled.
The switch is behavior preserving: both backends place grid points identically,
with the origin at the nuclear bounding box corner minus the margin and the step
equal to the padded extent divided by
n - 1along each axis, and both use thesame atomic orbital ordering.
GauXC's cube-generation support is now upstream, so the dependency moves off the
temporary fork and pins
wavefunction91/GauXCat merge commit162e4562552323a871af17ae4acd73b71071bd24.Two dependency patches were needed to build across platforms:
libaec1.1.6, retargeting the archive fromthe intermittently failing GitLab URL to libaec's canonical GitHub mirror.
Recipe and version are unchanged.
calls
exit()without including<stdlib.h>on that preprocessor path.Neither affects cube-generation behavior or adds a runtime dependency.
API