Add MaxDeterminantGrid with full AngularGrid and AtomGrid integration by alok-108 · Pull Request #297 · theochem/grid

alok-108 · 2026-03-26T13:59:48Z

Implemented MaxDeterminantGrid using maximum determinant (Fekete) points for spherical integration with positive weights and improved numerical stability.

Key Features

Integrated with AngularGrid and AtomGrid
- Supports both degree and size initialization
- Uses precomputed datasets

Validation

Verified sum weights = 4pi
- Constant integration returns 4pi
- Spherical harmonics validated across full range
- All tests pass (79/79)

Fixes

Corrected spherical harmonic test coverage
- Fixed Sphinx math formatting
- Added notebook integration via .nblink

Notes

All tests and documentation build successfully in a clean environment.

Updates (pre-review cleanup):

Removed debug artifacts (results.txt, output.txt) and auto-generated _version.py from tracked files
- Fixed hardcoded local path in fetch_maxdet_data.py
- Added __all__ to max_det.py, removed dead MAXDET_CACHE from angular.py
- Consolidated filesystem scanning into single _get_available_degrees() method
- Normalized weight convention: raw weights stored internally, 4pi factor applied consistently at AngularGrid level

)

…, fix tests and documentation

alok-108 · 2026-03-26T14:19:29Z

Hi @PaulWAyers,

Would really appreciate your review whenever you get time. Happy to make any changes if needed!

alok-108 · 2026-03-26T14:20:34Z

Hi @PaulWAyers,

Would really appreciate your review whenever you get time. Happy to make any changes if needed!

…ate loader

Copilot

Pull request overview

Adds a new maximum-determinant (Fekete) spherical quadrature (MaxDeterminantGrid) and wires it into AngularGrid/AtomGrid, with bundled precomputed datasets and docs/examples.

Changes:

Introduces MaxDeterminantGrid with loading of packaged .npz point/weight datasets and a convenience integrate() API.
Extends AngularGrid/AtomGrid to support method="maxdet" and updates related tests.
Adds packaged maxdet datasets, a fetch script, and documentation/notebook integration.

Reviewed changes

Copilot reviewed 13 out of 26 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/grid/max_det.py	New MaxDeterminantGrid implementation and dataset loader.
src/grid/angular.py	Adds `method="maxdet"` support and routes loading/degree-size mapping accordingly.
src/grid/atomgrid.py	Updates docstrings to include `maxdet` as an available angular method.
src/grid/init.py	Re-exports maxdet module from package root.
src/grid/tests/test_maxdet.py	New unit tests for maxdet weights and integration behavior.
src/grid/tests/test_atomgrid.py	Extends existing AtomGrid tests to run with `method="maxdet"`.
src/grid/tests/test_angular.py	Fixes/adjusts spherical harmonic indexing and assertions in tests.
src/grid/data/max_det/init.py	Marks maxdet data directory as a package.
src/grid/data/max_det/maxdet_1_4.npz	Adds precomputed maxdet dataset (degree 1).
src/grid/data/max_det/maxdet_2_9.npz	Adds precomputed maxdet dataset (degree 2).
src/grid/data/max_det/maxdet_3_16.npz	Adds precomputed maxdet dataset (degree 3).
src/grid/data/max_det/maxdet_4_25.npz	Adds precomputed maxdet dataset (degree 4).
src/grid/data/max_det/maxdet_5_36.npz	Adds precomputed maxdet dataset (degree 5).
src/grid/data/max_det/maxdet_10_121.npz	Adds precomputed maxdet dataset (degree 10).
src/grid/data/max_det/maxdet_20_441.npz	Adds precomputed maxdet dataset (degree 20).
src/grid/data/max_det/maxdet_30_961.npz	Adds precomputed maxdet dataset (degree 30).
src/grid/data/max_det/maxdet_40_1681.npz	Adds precomputed maxdet dataset (degree 40).
scripts/fetch_maxdet_data.py	Script to download and convert upstream maxdet datasets into `.npz`.
examples/max_det_integration.ipynb	Example notebook demonstrating maxdet spherical integration.
doc/notebooks/max_det_integration.nblink	Links the example notebook into the Sphinx docs build.
doc/index.rst	Adds the maxdet notebook to the documentation toctree.
MANIFEST.in	Ensures maxdet `.npz` files are included in source distributions.
.gitignore	Ignores debug/output artifacts and autogenerated version file.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-21T18:13:53Z

+            The integral of the function/arrays over the unit sphere.
+        """
+        if len(args) == 1 and callable(args[0]):
+            return 4 * np.pi * np.sum(args[0](self.points) * self.weights)


In the callable branch, integrate() bypasses Grid.integrate()'s shape/type checks and uses np.sum(args[0](self.points) * self.weights). If the callable returns a scalar or an array with shape (N,1)/(1,N), NumPy broadcasting can produce an incorrect result without raising. Consider coercing the result to np.asarray, validating it is 1-D with shape (self.size,), and then using the same einsum/integration path as Grid.integrate().

Suggested change

return 4 * np.pi * np.sum(args[0](self.points) * self.weights)

values = np.asarray(args[0](self.points))

if values.ndim != 1 or values.shape != (self.size,):

raise ValueError(

f"Callable passed to integrate must return a 1-D array with shape {(self.size,)}, "

f"got shape {values.shape}."

)

return 4 * np.pi * super().integrate(values)

Copilot · 2026-04-21T18:13:53Z

+import numpy as np
+import pytest
+from grid.max_det import MaxDeterminantGrid
+from grid.utils import generate_real_spherical_harmonics, convert_cart_to_sph


convert_cart_to_sph is imported but never used in this test file. Removing the unused import will keep the test module clean and avoid linting issues in stricter environments.

Suggested change

from grid.utils import generate_real_spherical_harmonics, convert_cart_to_sph

from grid.utils import generate_real_spherical_harmonics

…used imports

alok-108 · 2026-04-21T20:55:14Z

@PaulWAyers @marco-2023 @FarnazH

I fixed the three things the Copilot code review pointed out. Here’s what I changed:

Checking maxdet values early
In the AngularGrid._get_degree_and_size method, I now check the degree and size numbers right at the start. This way, wrong values for maxdet are caught immediately for all grid types, instead of being quietly accepted.
I also updated the test test_errors_and_warnings so it now expects a TypeError (an error about wrong data type).
Checking the shape of custom function results
When MaxDeterminantGrid.integrate uses a custom function, the output is turned into a one‑dimensional list of numbers. Then I check that its length matches self.size. After that, the normal integrate method handles it safely, without any hidden size‑mismatch problems.
Removed an unused import
I deleted the line from grid.utils import convert_cart_to_sph in test_maxdet.py because it wasn’t being used.

All the tests in test_angular.py and test_maxdet.py still pass on my computer. The changes are in commit 7cc7b95 and should show up here now.

alok-108 added 2 commits March 24, 2026 23:00

Add spherical integration using maximum determinant points (theochem#237

b1af716

)

Add MaxDeterminantGrid with full AngularGrid and AtomGrid integration…

d4b9c4e

…, fix tests and documentation

alok-108 mentioned this pull request Mar 26, 2026

Integration of the Sphere with Maximum Determinant Points #237

Closed

alok-108 mentioned this pull request Mar 26, 2026

GSoC 2026: Integration of the Sphere with Maximum Determinant Points #289

Open

alok-108 added 4 commits April 18, 2026 19:34

Pre-review cleanup: remove artifacts, fix weight convention, consolid…

2474ba2

…ate loader

Remove scratch files and fix duplicate assertion in test_atomgrid

b9f8d40

Remove hardcoded assertion in test_value_fitting

1ace207

Remove hardcoded assert len(spls) == 16 from test_value_fitting

961f284

PaulWAyers requested review from FarnazH, Copilot and marco-2023 and removed request for FarnazH and Copilot April 21, 2026 18:10

Copilot started reviewing on behalf of PaulWAyers April 21, 2026 18:10 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Address Copilot code review: Input validation, integrate() branch, un…

7cc7b95

…used imports

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add MaxDeterminantGrid with full AngularGrid and AtomGrid integration#297

Add MaxDeterminantGrid with full AngularGrid and AtomGrid integration#297
alok-108 wants to merge 7 commits intotheochem:masterfrom
alok-108:feature/max-det-spherical-grid

alok-108 commented Mar 26, 2026 •

edited

Loading

Uh oh!

alok-108 commented Mar 26, 2026

Uh oh!

alok-108 commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

alok-108 commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-            return 4 * np.pi * np.sum(args[0](self.points) * self.weights)
+            values = np.asarray(args[0](self.points))
+            if values.ndim != 1 or values.shape != (self.size,):
+                raise ValueError(
+                    f"Callable passed to integrate must return a 1-D array with shape {(self.size,)}, "
+                    f"got shape {values.shape}."
+                )
+            return 4 * np.pi * super().integrate(values)

	from grid.utils import generate_real_spherical_harmonics, convert_cart_to_sph
	from grid.utils import generate_real_spherical_harmonics

Conversation

alok-108 commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Features

Validation

Fixes

Notes

Updates (pre-review cleanup):

Uh oh!

alok-108 commented Mar 26, 2026

Uh oh!

alok-108 commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

alok-108 commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alok-108 commented Mar 26, 2026 •

edited

Loading