Conversation
ddudt
added
interface
desc/transform.py
Outdated
| 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.
|
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 | 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2053 +/- ##
==========================================
- Coverage 94.52% 94.25% -0.28%
==========================================
Files 102 107 +5
Lines 28785 29346 +561
==========================================
+ Hits 27210 27659 +449
- Misses 1575 1687 +112
🚀 New features to boost your workflow:
|
| r_grid = Grid( | ||
| map_coordinates( | ||
| eq, | ||
| r_grid.nodes, | ||
| ["rho", "theta", "phi"], | ||
| ["rho", "theta", "zeta"], | ||
| period=(np.inf, 2 * np.pi, 2 * np.pi), | ||
| guess=r_grid.nodes, | ||
| ), | ||
| sort=False, | ||
| ) |
There was a problem hiding this comment.
Was this code necessary or was it only anticipating the future change where
There was a problem hiding this comment.
I don't remember what was the reason to revert this but at some point in #1683 (comment) I removed this line, and in the dev meeting we changed our minds, can't remember why though
There was a problem hiding this comment.
Surely there was a reason though. I'd rather not change this now if we don't have a good reason for changing it at the moment
| Breaking Changes | ||
|
|
||
| - Restructures the grid classes to allow for new grids in different coordinate systems besides flux coordinates. The old grid classes are aliased to the new grids of type ``AbstractGridFlux`` and are backwards compatable with the new API. ``Curve`` objects now require a compute grid of type ``AbstractGridCurve``, and ``FourierRZToroidalSurface`` objects now require a compute grid of type ``AbstractGridSurface``. | ||
|
|
There was a problem hiding this comment.
We should release a new version of DESC after this is merged, since it does change the API for things like coil optimizations and REGCOIL runs.
There was a problem hiding this comment.
add deprecation warnings for curve and surface as well
6 participants
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 ABCAbstractGridFluxis 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, AbstractGridCurve)checks wherever appropriateCurveobjectsFourierRZToroidalSurfaceobjectsGrid->CustomGridFlux, etc. (do not deprecate)