New Grid API by ddudt · Pull Request #2053 · PlasmaControl/DESC

ddudt · 2026-01-07T00:01:58Z

Resolves #1840

Restructures and generalizes the Grid classes. The new ABC AbstractGrid that all other grids inherit from does not assume a particular coordinate system. The new ABC AbstractGridFlux is specific to ($\rho,\theta,\zeta$) coordinates and is the parent class of all existing grid classes in flux coordinates. Most of the changes involved moving existing code into one of these new ABCs.

The goal is to allow for new grid classes in the future that represent other coordinate systems, without changing the existing public API.

To-Do:

Ensure tests pass
Generalize Basis dimensions (no references to flux coords)
Include isinstance(grid, AbstractGridCurve) checks wherever appropriate
Add 1D grids for Curve objects
Add 2D grids for FourierRZToroidalSurface objects
Alias flux grid class names like Grid -> CustomGridFlux, etc. (do not deprecate)

ddudt · 2026-01-07T00:06:08Z

            and self.grid.NFP != self.basis.NFP
            and self.basis.N != 0
-            and grid.node_pattern != "custom"
+            and not isinstance(grid, Grid)


This is equivalent to the old code, since the old Grid class was hard-coded to have node_pattern="custom". In the new code, node_pattern is only an attribute of the ConcentricGrid class.

dpanici · 2026-01-07T20:13:43Z

Add checks in objectives and inside geometric/magneticfield/coil compute methods to ensure grids passed in are correct (i.e. 1D for a filament coil, 3D for finite build coil, 1D or 2D rtz for surface)

dpanici · 2026-01-07T20:20:49Z

Rename AbstractRTZGrid to AbstractGridFlux instead of RTZ

github-actions · 2026-01-07T21:29:30Z

Memory benchmark result

|               Test Name                |      %Δ      |    Master (MB)     |      PR (MB)       |    Δ (MB)    |    Time PR (s)     |  Time Master (s)   |
| -------------------------------------- | ------------ | ------------------ | ------------------ | ------------ | ------------------ | ------------------ |
  test_objective_jac_w7x                 |    1.92 %    |     3.843e+03      |     3.917e+03      |    73.83     |       38.77        |       35.84        |
  test_proximal_jac_w7x_with_eq_update   |   -0.75 %    |     6.614e+03      |     6.564e+03      |    -49.76    |       160.17       |       158.04       |
  test_proximal_freeb_jac                |    0.12 %    |     1.322e+04      |     1.323e+04      |    16.08     |       83.02        |       80.99        |
  test_proximal_freeb_jac_blocked        |   -0.32 %    |     7.564e+03      |     7.540e+03      |    -24.30    |       72.80        |       71.30        |
  test_proximal_freeb_jac_batched        |    0.20 %    |     7.479e+03      |     7.494e+03      |    15.05     |       72.90        |       73.29        |
  test_proximal_jac_ripple               |   -2.18 %    |     3.631e+03      |     3.552e+03      |    -79.10    |       63.47        |       64.59        |
  test_proximal_jac_ripple_bounce1d      |   -1.00 %    |     3.548e+03      |     3.512e+03      |    -35.59    |       74.33        |       74.43        |
  test_eq_solve                          |   -1.13 %    |     2.034e+03      |     2.011e+03      |    -23.03    |       92.25        |       91.33        |

For the memory plots, go to the summary of Memory Benchmarks workflow and download the artifact.

codecov · 2026-01-07T22:08:51Z

Codecov Report

❌ Patch coverage is 87.59287% with 167 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.21%. Comparing base (f8c0ec0) to head (cc782ec).

Files with missing lines	Patch %	Lines
desc/grid/surface.py	80.80%	43 Missing ⚠️
desc/grid/flux.py	77.90%	38 Missing ⚠️
desc/grid/curve.py	75.35%	35 Missing ⚠️
desc/grid/core.py	94.17%	11 Missing ⚠️
desc/geometry/core.py	71.42%	8 Missing ⚠️
desc/objectives/_geometry.py	83.33%	7 Missing ⚠️
desc/external/terpsichore.py	0.00%	6 Missing ⚠️
desc/plotting.py	93.93%	6 Missing ⚠️
desc/coils.py	88.88%	2 Missing ⚠️
desc/grid/utils.py	98.56%	2 Missing ⚠️
... and 7 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2053      +/-   ##
==========================================
- Coverage   94.48%   94.21%   -0.27%     
==========================================
  Files         102      107       +5     
  Lines       28721    29280     +559     
==========================================
+ Hits        27137    27587     +450     
- Misses       1584     1693     +109

Files with missing lines	Coverage Δ
desc/basis.py	`97.74% <100.00%> (ø)`
desc/compat.py	`88.61% <100.00%> (ø)`
desc/compute/_geometry.py	`98.47% <100.00%> (+0.37%)`	⬆️
desc/compute/_neoclassical.py	`100.00% <ø> (ø)`
desc/compute/_old.py	`100.00% <ø> (ø)`
desc/compute/data_index.py	`95.89% <ø> (ø)`
desc/compute/utils.py	`97.71% <100.00%> (+0.01%)`	⬆️
desc/equilibrium/initial_guess.py	`93.79% <100.00%> (ø)`
desc/geometry/curve.py	`96.44% <100.00%> (ø)`
desc/geometry/surface.py	`97.67% <100.00%> (ø)`
... and 38 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

… dd/grids

unalmis · 2026-02-26T23:38:22Z

If this PR gets merged before the older PR #1919 , then please address the merge conflicts that this PR #2053 introduces.

ddudt · 2026-03-09T20:46:46Z

Presently we cannot do the following, because it throws an error that a variable is used before it is defined:

from desc import grid
grid = grid.Grid()

The easy solution is to use a different variable name other than grid, but it is a little annoying. I know we wanted to keep our API the same, but this would be another argument in favor of switching the submodule name to desc.grids (plural) instead of desc.grid.

f0uriest · 2026-03-09T21:01:08Z

Presently we cannot do the following, because it throws an error that a variable is used before it is defined:
from desc import grid
grid = grid.Grid()
The easy solution is to use a different variable name other than grid, but it is a little annoying. I know we wanted to keep our API the same, but this would be another argument in favor of switching the submodule name to desc.grids (plural) instead of desc.grid.

Does this work on master? I feel like it should behave the same. I don't think I've ever imported the grid module by itself though, only desc.grid

dpanici · 2026-03-12T17:31:45Z

Presently we cannot do the following, because it throws an error that a variable is used before it is defined:
from desc import grid
grid = grid.Grid()
The easy solution is to use a different variable name other than grid, but it is a little annoying. I know we wanted to keep our API the same, but this would be another argument in favor of switching the submodule name to desc.grids (plural) instead of desc.grid.
Does this work on master? I feel like it should behave the same. I don't think I've ever imported the grid module by itself though, only desc.grid

this works on master

from desc import grid

g = grid.LinearGrid()

f0uriest · 2026-03-12T17:45:26Z

Presently we cannot do the following, because it throws an error that a variable is used before it is defined:
from desc import grid
grid = grid.Grid()
The easy solution is to use a different variable name other than grid, but it is a little annoying. I know we wanted to keep our API the same, but this would be another argument in favor of switching the submodule name to desc.grids (plural) instead of desc.grid.
Does this work on master? I feel like it should behave the same. I don't think I've ever imported the grid module by itself though, only desc.grid
this works on master
from desc import grid

g = grid.LinearGrid()

This also seems to work on this branch:

from desc import grid
grid = grid.LinearGrid()

@ddudt are you sure this is an issue?

YigitElma · 2026-03-12T18:41:30Z

If you try to create a new grid.Grid for example, the above code will behave badly, because it replaces the module with an object instance. The suggestion we discussed in dev meeting was to use

from desc import grid as grid_module

grid = grid_module.Grid()

YigitElma · 2026-04-06T03:14:30Z

        safenorm(data["e_theta"], axis=-1),
        line_label="theta",
-        fix_surface=("rho", max_rho),
+        fix_surface=("x0", 0.0),


Why this is 0?

YigitElma · 2026-04-06T03:19:31Z

    """
-    line_label = grid.get_label(line_label)
+    line_label = grid.get_label(line_label)  # general coordinate -> x0, x1, or x2
    fix_label = grid.get_label(fix_surface[0])


Should the default of fix_surface change? Currently it is ("rho", 1.0)

YigitElma · 2026-04-06T03:34:49Z

-        quantities on `LinearGrid(rho=rho, theta=Np, zeta=Nt, NFP=1, endpoint=True)`
-        and add it to the mesh by `mesh['name'] = value`. Once the mesh is changed,
+        Created structured grid object. With this object one can compute more quantities
+        and add them to the mesh by `mesh['name'] = value`. Once the mesh is changed,


I prefer to keep the default grid info since it needs to be the same if someone wants to add data to the mesh

YigitElma · 2026-04-06T04:13:15Z

+            del self._inverse_theta_idx
+        if hasattr(self, "_inverse_poloidal_idx"):
+            self._inverse_x1_idx = self._inverse_poloidal_idx
+            del self._inverse_theta_idx


Suggested change

del self._inverse_theta_idx

del self._inverse_poloidal_idx

YigitElma · 2026-04-06T04:21:15Z

+        if coordinates[1] == "a":
+            period = (np.inf, np.inf, 2 * np.pi / NFP)
+        else:
+            period(np.inf, 2 * np.pi, 2 * np.pi / NFP)


Suggested change

period(np.inf, 2 * np.pi, 2 * np.pi / NFP)

period = (np.inf, 2 * np.pi, 2 * np.pi / NFP)

?

YigitElma · 2026-04-06T04:38:57Z

+        ) = self._find_unique_inverse_nodes()
+        self._weights = self._scale_weights()
+
+    def _create_nodes(  # noqa: C901


This function and all create_nodes methods of linear grid types are repeated code, right? Would it be better to have some utility functions for theta, zeta parts?

YigitElma · 2026-04-06T04:42:48Z

            Biot-Savart. Should NOT include endpoint at 2pi.
-        vc_source_grid : LinearGrid
-            LinearGrid to use for the singular integral for the virtual casing
+        vc_source_grid : LinearGridFlux


Suggested change

vc_source_grid : LinearGridFlux

vc_source_grid : LinearGridToroidalSurface

YigitElma · 2026-04-06T04:43:02Z

-        vc_source_grid : LinearGrid
-            LinearGrid to use for the singular integral for the virtual casing
+        vc_source_grid : LinearGridFlux
+            LinearGridFlux to use for the singular integral for the virtual casing


Suggested change

LinearGridFlux to use for the singular integral for the virtual casing

LinearGridToroidalSurface to use for the singular integral for the virtual casing

YigitElma · 2026-04-06T04:48:10Z

-        LinearGrid to use for the singular integral for the virtual casing
+        ``LinearGridToroidalSurface(M=max(3 * current_potential_field.M_Phi, 30),
+        N=max(3 * current_potential_field.N_Phi, 30), NFP=eq.NFP)``
+    vc_source_grid : LinearGridFlux


Suggested change

vc_source_grid : LinearGridFlux

vc_source_grid : LinearGridToroidalSurface

YigitElma · 2026-04-06T04:51:08Z

        default_M = 2 * eq.M if not hasattr(eq, "_M_grid") else eq.M_grid
        default_N = 2 * eq.N if not hasattr(eq, "_M_grid") else eq.N_grid
-        plasma_grid = self._plasma_grid or LinearGrid(
+        plasma_grid = self._plasma_grid or LinearGridFlux(


Suggested change

plasma_grid = self._plasma_grid or LinearGridFlux(

plasma_grid = self._plasma_grid or LinearGridToroidalSurface(

YigitElma · 2026-04-06T04:52:50Z

Github acts buggy for this PR (probably due to high number of changes), I will review the rest later

YigitElma

Do we eventually want to always enforce the proper grid type (no matter if the computation is the same for both)? I think that is slightly annoying, because for example, now we have a dedicated curve grid classes but we don't use them for magnetic axis. Or we have a toroidal surface class, but using them doesn't make the computation any different.

If you can explain which grid should be used when, and when it will be enforced in the grid notebook, it would help a lot for other reviewers, too.

Still need to review 40% of the files :)

YigitElma · 2026-04-14T20:39:05Z

        else:
            grid = self._grid

+        errorif(


There will be couple hundred of this error in many places, might consider a helper function

# inside desc.utils or somewhere def errorif_wrong_grid(grid, type, grid_name="grid"): errorif( not isinstance(grid, type), ValueError, msg=f"{grid_name} must be of type {type.__name__}, but got type {type(grid)}.", )

YigitElma · 2026-04-14T22:01:36Z

        # the sign convention is positive poloidal current flows up through the torus
        # hole
-        grid_at_surf = LinearGrid(rho=1.0, M=eq.M_grid, N=eq.N_grid)
+        grid_at_surf = LinearGridFlux(rho=1.0, M=eq.M_grid, N=eq.N_grid)


Suggested change

grid_at_surf = LinearGridFlux(rho=1.0, M=eq.M_grid, N=eq.N_grid)

grid_at_surf = LinearGridToroidalSurface(rho=1.0, M=eq.M_grid, N=eq.N_grid)

YigitElma · 2026-04-14T22:02:49Z

        dphi = phi[1] - phi[0]

-        grid = LinearGrid(M=0, L=0, zeta=phi, NFP=nfp)
+        grid = LinearGridFlux(M=0, L=0, zeta=phi, NFP=nfp)


Suggested change

grid = LinearGridFlux(M=0, L=0, zeta=phi, NFP=nfp)

grid = LinearGridCurve(M=0, L=0, zeta=phi, NFP=nfp)

?

YigitElma · 2026-04-14T22:02:55Z

        Zbasis_sin = FourierSeries(N=N, NFP=nfp, sym=False)

-    grid = LinearGrid(M=0, L=0, zeta=phi, NFP=nfp)
+    grid = LinearGridFlux(M=0, L=0, zeta=phi, NFP=nfp)


Suggested change

grid = LinearGridFlux(M=0, L=0, zeta=phi, NFP=nfp)

grid = LinearGridCurve(M=0, L=0, zeta=phi, NFP=nfp)

?

YigitElma · 2026-04-14T22:03:00Z

    bases["Zbasis_sin"] = Zbasis_sin

-    grid = LinearGrid(M=0, L=0, zeta=phi, NFP=nfp)
+    grid = LinearGridFlux(M=0, L=0, zeta=phi, NFP=nfp)


Suggested change

grid = LinearGridFlux(M=0, L=0, zeta=phi, NFP=nfp)

grid = LinearGridCurve(M=0, L=0, zeta=phi, NFP=nfp)

?

YigitElma · 2026-04-14T22:20:31Z

            # need some sensible default grid
            if self._grid is None:
-                grid = LinearGrid(M=eq.M_grid, N=eq.N_grid, NFP=eq.NFP, sym=eq.sym)
+                grid = LinearGridFlux(M=eq.M_grid, N=eq.N_grid, NFP=eq.NFP, sym=eq.sym)


These are also better be LienarGridToroidalSurface

YigitElma · 2026-04-14T22:25:03Z

        eq,
-        eval_grid=LinearGrid(M=20, N=20, NFP=eq.NFP, sym=True),
-        source_grid=LinearGrid(M=40, N=40, NFP=eq.NFP),
+        eval_grid=LinearGridFlux(M=20, N=20, NFP=eq.NFP, sym=True),


Suggested change

eval_grid=LinearGridFlux(M=20, N=20, NFP=eq.NFP, sym=True),

eval_grid=LinearGridToroidalSurface(M=20, N=20, NFP=eq.NFP, sym=True),

YigitElma · 2026-04-14T22:25:15Z

        eq,
-        eval_grid=LinearGrid(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),
-        source_grid=LinearGrid(M=M_sgrid, N=N_sgrid, NFP=eq.NFP),
+        eval_grid=LinearGridFlux(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),


Suggested change

eval_grid=LinearGridFlux(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),

eval_grid=LinearGridToroidalSurface(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),

YigitElma · 2026-04-14T22:26:06Z

        eq,
-        eval_grid=LinearGrid(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),
-        source_grid=LinearGrid(M=M_sgrid, N=N_sgrid, NFP=eq.NFP),
+        eval_grid=LinearGridFlux(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),


Suggested change

eval_grid=LinearGridFlux(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),

eval_grid=LinearGridToroidalSurface(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),

YigitElma · 2026-04-14T22:26:15Z

        eq,
-        eval_grid=LinearGrid(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),
-        source_grid=LinearGrid(M=M_sgrid, N=N_sgrid, NFP=eq.NFP),
+        eval_grid=LinearGridFlux(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),


Suggested change

eval_grid=LinearGridFlux(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),

eval_grid=LinearGridToroidalSurface(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),

daniel-dudt added 8 commits August 14, 2025 15:29

rename _Grid to AbstractGrid

441cb57

remove functionality from AbstractGrid

6ef6c6f

merge with master

43ab00e

reorganize grid file to subdirectory

c2f0011

update flux grids

6587ee1

test_grid is passing

ebf39af

fix references in surface_integrals

bace610

edit Changelog

8ac82d2

ddudt self-assigned this Jan 7, 2026

ddudt commented Jan 7, 2026

View reviewed changes

Comment thread tests/test_grid.py

ddudt commented Jan 7, 2026

View reviewed changes

Comment thread desc/grid/core.py

ddudt commented Jan 7, 2026

View reviewed changes

Comment thread desc/grid/core.py

ddudt commented Jan 7, 2026

View reviewed changes

Comment thread desc/grid/flux.py

daniel-dudt added 5 commits January 6, 2026 19:31

fix LinearGrid resolution getters

afaa319

set default surface_label='rho'

1d17f22

fix typo in previous commit

6b410dc

add source_grid back

a6a816d

revert removal of source_grid test

4102387

daniel-dudt and others added 2 commits January 7, 2026 15:58

repairing tests

1fb537f

Merge branch 'master' into dd/grids

4bbe81d

ddudt commented Jan 7, 2026

View reviewed changes

Comment thread desc/grid/core.py Outdated

rename AbstractRTZGrid to AbstractGridFlux

545c063

daniel-dudt and others added 13 commits February 6, 2026 15:50

Merge branch 'dd/grids' of https://github.com/PlasmaControl/DESC into…

9ea8bb2

… dd/grids

no sym for curve grids

b691b74

resolve merge conflicts with master

2007c11

new test comparing linear grids

70b5cdc

fix line_integrals to work with spacing=0

16321e9

type switch in plot_boundaries

f589a97

do not plot surface with plot_surfaces

eac486c

fix plots in notebooks again

5884b89

replace errors with deprecation warnings

ea51f67

error if geometry classes do not compute on a grid

8573ae5

Merge branch 'master' into dd/grids

684ca04

Merge branch 'master' into dd/grids

6e9559e

Merge branch 'master' into dd/grids

1510a29

merge with master

4e24756

YigitElma added the hackathon Stuff to work on during hackathon label Mar 19, 2026

ddudt and others added 5 commits March 23, 2026 19:34

Merge branch 'master' into dd/grids

65b8100

Merge branch 'master' into dd/grids

6be4f9f

fix grid for constant_offset_surface

336906c

Merge branch 'master' into dd/grids

e09bdb1

Merge branch 'master' into dd/grids

cc782ec

YigitElma reviewed Apr 6, 2026

View reviewed changes

unalmis mentioned this pull request Apr 10, 2026

Add fft grid and raz grid to test against master #2157

Open

YigitElma requested changes Apr 14, 2026

View reviewed changes

	period(np.inf, 2 * np.pi, 2 * np.pi / NFP)
	period = (np.inf, 2 * np.pi, 2 * np.pi / NFP)

	vc_source_grid : LinearGridFlux
	vc_source_grid : LinearGridToroidalSurface

	LinearGridFlux to use for the singular integral for the virtual casing
	LinearGridToroidalSurface to use for the singular integral for the virtual casing

	plasma_grid = self._plasma_grid or LinearGridFlux(
	plasma_grid = self._plasma_grid or LinearGridToroidalSurface(

	grid_at_surf = LinearGridFlux(rho=1.0, M=eq.M_grid, N=eq.N_grid)
	grid_at_surf = LinearGridToroidalSurface(rho=1.0, M=eq.M_grid, N=eq.N_grid)

	grid = LinearGridFlux(M=0, L=0, zeta=phi, NFP=nfp)
	grid = LinearGridCurve(M=0, L=0, zeta=phi, NFP=nfp)

	eval_grid=LinearGridFlux(M=20, N=20, NFP=eq.NFP, sym=True),
	eval_grid=LinearGridToroidalSurface(M=20, N=20, NFP=eq.NFP, sym=True),

	eval_grid=LinearGridFlux(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),
	eval_grid=LinearGridToroidalSurface(M=M_egrid, N=N_egrid, NFP=eq.NFP, sym=True),

Conversation

ddudt commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dpanici commented Jan 7, 2026

Uh oh!

dpanici commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Memory benchmark result

Uh oh!

Uh oh!

codecov bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

unalmis commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ddudt commented Mar 9, 2026

Uh oh!

f0uriest commented Mar 9, 2026

Uh oh!

dpanici commented Mar 12, 2026

Uh oh!

f0uriest commented Mar 12, 2026

Uh oh!

YigitElma commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YigitElma commented Apr 6, 2026

Uh oh!

YigitElma left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ddudt commented Jan 7, 2026 •

edited

Loading

dpanici commented Jan 7, 2026 •

edited

Loading

github-actions bot commented Jan 7, 2026 •

edited

Loading

codecov bot commented Jan 7, 2026 •

edited

Loading

unalmis commented Feb 26, 2026 •

edited

Loading

YigitElma left a comment •

edited

Loading