Skip to content

Fix Grid.moments Cartesian orders on 1D grids - #338

Draft
PaulWAyers with Copilot wants to merge 3 commits into
masterfrom
copilot/fix-grid-moments-cartesian-1d
Draft

PaulWAyers with Copilot wants to merge 3 commits into
masterfrom
copilot/fix-grid-moments-cartesian-1d

Conversation

Copilot AI commented Sep 30, 2026 •

Copy link
Copy Markdown

Grid.moments(..., type_mom="cartesian") raised AttributeError: module 'numpy' has no attribute 'int' on grids with 1D point arrays (shape (N, 1)), since np.int was removed in NumPy 1.24. The dim == 1 branch also returned the wrong shape (arange(0, order+1) instead of a single row), which would have broken np.vstack in Grid.moments even with a valid dtype.

  • generate_orders_horton_order (src/grid/utils.py): dim == 1 branch now returns np.array([[order]]), matching the row shape produced by the dim == 2 and dim == 3 branches.
  • Tests: added coverage for generate_orders_horton_order at dim=1/2/3, and a test_moments_cartesian case exercised across 1D/2D/3D Grid point arrays.
import numpy as np
from grid.basegrid import Grid

pts = np.linspace(-1, 1, 5)[:, None]
grid = Grid(pts, np.full(5, 0.4))
grid.moments(2, np.array([[0.0]]), pts[:, 0] ** 2, "cartesian")  # now returns correctly shaped moments

Copilot AI and others added 2 commits September 30, 2026 11:55
Co-authored-by: PaulWAyers <15361871+PaulWAyers@users.noreply.github.com>
Co-authored-by: PaulWAyers <15361871+PaulWAyers@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix Grid.moments for Cartesian orders on 1D grids Fix Grid.moments Cartesian orders on 1D grids Sep 30, 2026
Copilot AI requested a review from PaulWAyers September 30, 2026 11:56
@PaulWAyers
PaulWAyers requested a balanced review from Copilot September 30, 2026 11:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation matches the documented order semantics and includes targeted regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes Cartesian moment-order generation for 1D grids while preserving multidimensional behavior.

Changes:

  • Returns correctly shaped Cartesian orders for 1D grids.
  • Adds utility and integration-level regression tests across 1D–3D grids.
  • Ignores setuptools-scm’s generated version file.
File Description
src/​grid/​utils.py Corrects 1D Cartesian order generation.
src/​grid/​tests/​test_utils.py Tests Cartesian orders across dimensions.
src/​grid/​tests/​test_grid.py Tests Cartesian moments on 1D–3D grids.
.gitignore Excludes generated version metadata.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Grid.moments with Cartesian orders fails on 1D grids

3 participants