New Grid API #2053
New Grid API #2053
Conversation
ddudt
added
interface
| and self.grid.NFP != self.basis.NFP | ||
| and self.basis.N != 0 | ||
| and grid.node_pattern != "custom" | ||
| and not isinstance(grid, Grid) |
There was a problem hiding this comment.
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.
| ) | ||
| def bounds(self): | ||
| """Bounds of coordinates.""" | ||
| return ((0, 1), (0, 2 * np.pi), (0, 2 * np.pi)) |
There was a problem hiding this comment.
The toroidal bound is
|
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) |
|
Rename AbstractRTZGrid to AbstractGridFlux instead of RTZ |
Memory benchmark result| Test Name | %Δ | Master (MB) | PR (MB) | Δ (MB) | Time PR (s) | Time Master (s) |
| -------------------------------------- | ------------ | ------------------ | ------------------ | ------------ | ------------------ | ------------------ |
test_objective_jac_w7x | -0.50 % | 4.033e+03 | 4.013e+03 | -20.25 | 40.97 | 37.67 |
test_proximal_jac_w7x_with_eq_update | 1.69 % | 6.506e+03 | 6.616e+03 | 109.96 | 165.34 | 164.95 |
test_proximal_freeb_jac | -0.17 % | 1.321e+04 | 1.319e+04 | -21.93 | 88.77 | 86.16 |
test_proximal_freeb_jac_blocked | 0.09 % | 7.530e+03 | 7.536e+03 | 6.49 | 76.94 | 74.81 |
test_proximal_freeb_jac_batched | -0.35 % | 7.500e+03 | 7.474e+03 | -26.36 | 76.44 | 75.43 |
test_proximal_jac_ripple | -1.76 % | 3.482e+03 | 3.421e+03 | -61.16 | 69.44 | 67.92 |
test_proximal_jac_ripple_bounce1d | -0.80 % | 3.588e+03 | 3.559e+03 | -28.87 | 82.30 | 79.68 |
test_eq_solve | 1.61 % | 2.001e+03 | 2.034e+03 | 32.29 | 97.01 | 96.62 |For the memory plots, go to the summary of |
| x = x.reshape(shape, order="F") | ||
| return x | ||
|
|
||
| def copy_data_from_other(self, x, other_grid, surface_label="rho", tol=1e-14): |
There was a problem hiding this comment.
I don't like having a default surface_label="rho" now that this ABC is for general coordinates. But this was the previous default, so it would break old code if we changed or removed it. Same for the compress and expand methods above.
This is still likely the most common use case, so I'm fine with keeping it as the default even though it feels awkward.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2053 +/- ##
==========================================
- Coverage 95.75% 95.69% -0.07%
==========================================
Files 102 105 +3
Lines 28344 28467 +123
==========================================
+ Hits 27141 27241 +100
- Misses 1203 1226 +23
🚀 New features to boost your workflow:
|
Resolves #1840
Restructures and generalizes the Grid classes. The new ABC$\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.
AbstractGridthat all other grids inherit from does not assume a particular coordinate system. The new ABCAbstractRTZGridis specific to (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:
Basisdimensions (no references to flux coords)isinstance(grid, AbstractRTZGrid)checks wherever appropriateGrid->RTZGrid, etc.