CoilSet with NFP and sym #864

ddudt · 2024-02-07T23:17:02Z

Resolves #858

TODO:

Get coil set working with multiple field period symmetry (NFP > 1)
Get coil set working with stellarator symmetyr (sym = True)
Get it working in both $(R,\phi,Z)$ and $(X,Y,Z)$ coordinates

github-actions · 2024-02-07T23:47:53Z

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_lowres         |     +0.41 +/- 1.58     | +5.03e-05 +/- 1.95e-04 |  1.24e-02 +/- 1.2e-04  |  1.24e-02 +/- 1.5e-04  |
 test_build_transform_fft_midres         |     +0.80 +/- 1.48     | +7.30e-04 +/- 1.35e-03 |  9.19e-02 +/- 7.1e-04  |  9.12e-02 +/- 1.1e-03  |
 test_build_transform_fft_highres        |     +0.97 +/- 0.96     | +4.46e-03 +/- 4.43e-03 |  4.65e-01 +/- 4.1e-03  |  4.61e-01 +/- 1.6e-03  |
 test_equilibrium_init_lowres            |     +0.64 +/- 2.57     | +5.06e-03 +/- 2.03e-02 |  7.94e-01 +/- 1.7e-02  |  7.89e-01 +/- 1.1e-02  |
 test_equilibrium_init_medres            |     +0.21 +/- 1.79     | +3.01e-03 +/- 2.55e-02 |  1.43e+00 +/- 1.9e-02  |  1.42e+00 +/- 1.7e-02  |
 test_equilibrium_init_highres           |     -0.44 +/- 1.21     | -1.86e-02 +/- 5.08e-02 |  4.18e+00 +/- 2.2e-02  |  4.20e+00 +/- 4.6e-02  |
 test_objective_compile_dshape_current   |     -4.54 +/- 7.56     | -2.07e-01 +/- 3.45e-01 |  4.36e+00 +/- 2.5e-01  |  4.57e+00 +/- 2.4e-01  |
 test_objective_compile_atf              |     -0.07 +/- 5.59     | -6.52e-03 +/- 5.20e-01 |  9.31e+00 +/- 4.1e-01  |  9.31e+00 +/- 3.3e-01  |
 test_objective_compute_dshape_current   |     -0.25 +/- 3.28     | -5.37e-06 +/- 7.00e-05 |  2.13e-03 +/- 3.8e-05  |  2.13e-03 +/- 5.9e-05  |
 test_objective_compute_atf              |     +0.87 +/- 1.26     | +6.47e-05 +/- 9.34e-05 |  7.48e-03 +/- 4.6e-05  |  7.42e-03 +/- 8.1e-05  |
 test_objective_jac_dshape_current       |     +1.74 +/- 8.49     | +7.76e-04 +/- 3.79e-03 |  4.55e-02 +/- 3.3e-03  |  4.47e-02 +/- 1.9e-03  |
 test_objective_jac_atf                  |     -1.47 +/- 4.29     | -3.31e-02 +/- 9.67e-02 |  2.22e+00 +/- 7.8e-02  |  2.26e+00 +/- 5.7e-02  |
 test_perturb_1                          |     -2.77 +/- 12.12    | -2.44e-01 +/- 1.07e+00 |  8.58e+00 +/- 8.1e-01  |  8.82e+00 +/- 7.0e-01  |
 test_perturb_2                          |     -1.07 +/- 6.55     | -1.55e-01 +/- 9.51e-01 |  1.44e+01 +/- 4.5e-01  |  1.45e+01 +/- 8.4e-01  |

codecov · 2024-02-09T23:43:15Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e246e90) 94.81% compared to head (c38cb66) 94.84%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #864      +/-   ##
==========================================
+ Coverage   94.81%   94.84%   +0.02%     
==========================================
  Files          80       80              
  Lines       19695    19711      +16     
==========================================
+ Hits        18674    18695      +21     
+ Misses       1021     1016       -5

Files	Coverage Δ
desc/coils.py	`98.50% <100.00%> (+0.38%)`	⬆️
desc/compute/geom_utils.py	`100.00% <ø> (ø)`
desc/equilibrium/equilibrium.py	`96.31% <100.00%> (ø)`
desc/magnetic_fields.py	`96.79% <100.00%> (+0.72%)`	⬆️

... and 1 file with indirect coverage changes

desc/magnetic_fields.py

desc/coils.py

ddudt · 2024-02-10T00:00:47Z

desc/coils.py

-        def body(B, x):
-            B += self[0].compute_magnetic_field(
-                coords, params=x, basis=basis, grid=grid
+        # stellarator symmetry is easiest in [X,Y,Z] coordinates


I found it was easiest to convert back and forth between the two different coordinate systems to use whichever one is most convenient in the moment, rather than doing all the computations in whichever basis the user input. These conversions are small constant matrix multiplications so I figure there isn't much downside to this approach.

ddudt · 2024-02-10T00:02:30Z

tests/test_coils.py

+        B_xyz = coil.compute_magnetic_field(
+            grid_xyz, basis="xyz", source_grid=coil_grid
+        )
+        B_rpz = coil.compute_magnetic_field(
+            grid_rpz, basis="rpz", source_grid=coil_grid
+        )


I realized we didn't have any test to check that compute_magnetic_field(basis="rpz") gives the same result as compute_magnetic_field(basis="xyz"), so I added it here for each coil.

ddudt · 2024-02-10T00:08:31Z

tests/test_coils.py

        np.testing.assert_allclose(B_true, B_approx, rtol=1e-3, atol=1e-10)

+    @pytest.mark.unit
+    def test_symmetry_magnetic_field(self):


Summary of what this new test is checking:

sym_coilset contains only 3 unique coils in half of one field period. It computes it's magnetic field assuming field period symmetry and stellarator symmetry, using the new code that was added.

asym_coilset contains 24 coils but should be equivalent to the other coil set (3 unique coils, x2 for stellarator symmetry, x4 field periods). It computes it's magnetic field the "old school" way from each coil.

This test checks that both coil sets give the same magnetic fields, in both coordinate systems.

desc/coils.py

desc/magnetic_fields.py

tests/test_coils.py

desc/coils.py

dpanici · 2024-02-14T20:36:52Z

Change back to coords but make sure it cant accept Grid for that argument

made requested changes

desc/coils.py

desc/equilibrium/equilibrium.py

- deleted eq arg in build() - used wrapper for Bplasma - updated grid to source_grid for compute_magnetic_field - add tolerances for testing axisymmetric Bnorm (maybe #864)

daniel-dudt added 4 commits February 7, 2024 10:56

add NFP and sym attrs to CoilSet

f4a311b

error handling when creating CoilSet

6b30dbd

add template for new test

c71e8ca

CoilSet working with multiple field periods

1d88077

ddudt added the coil stuff relating to coils and coil optimization label Feb 7, 2024

ddudt self-assigned this Feb 7, 2024

daniel-dudt added 3 commits February 7, 2024 21:47

merge with master

0409367

cleanup

1e75ab2

test passing with NFP>1

2b3669e

works with stellarator symmetry

e61adc5

ddudt commented Feb 9, 2024

View reviewed changes

desc/magnetic_fields.py Outdated Show resolved Hide resolved

ddudt commented Feb 9, 2024

View reviewed changes

desc/coils.py Outdated Show resolved Hide resolved

ddudt commented Feb 10, 2024

View reviewed changes

ddudt marked this pull request as ready for review February 10, 2024 00:08

ddudt requested review from f0uriest and dpanici February 10, 2024 00:08

f0uriest previously requested changes Feb 10, 2024

View reviewed changes

ddudt and others added 6 commits February 12, 2024 14:02

Merge branch 'master' into dd/coilset

12bc6fe

rename to 'eval_grid'

eba41f8

update CoilSet init documentation

bbd6f81

update CoilSet.from_symmetry documentation

99b6767

Merge branch 'master' into dd/coilset

fe837a4

Merge branch 'master' into dd/coilset

e272ccb

ddudt requested a review from f0uriest February 13, 2024 16:39

dpanici reviewed Feb 14, 2024

View reviewed changes

desc/coils.py Outdated Show resolved Hide resolved

daniel-dudt added 2 commits February 14, 2024 15:11

don't allow coords to be a Grid

1492da7

replace python for loop with fori_loop

c38cb66

ddudt requested review from dpanici and f0uriest and removed request for f0uriest February 14, 2024 22:36

f0uriest approved these changes Feb 15, 2024

View reviewed changes

ddudt merged commit 9d5de57 into master Feb 15, 2024
17 checks passed

ddudt deleted the dd/coilset branch February 15, 2024 05:29

dpanici reviewed Feb 15, 2024

View reviewed changes

desc/coils.py Show resolved Hide resolved

dpanici reviewed Feb 15, 2024

View reviewed changes

desc/coils.py Show resolved Hide resolved

dpanici reviewed Feb 15, 2024

View reviewed changes

desc/equilibrium/equilibrium.py Show resolved Hide resolved

kianorr added a commit that referenced this pull request Feb 27, 2024

address some reviews

b8c2070

- deleted eq arg in build() - used wrapper for Bplasma - updated grid to source_grid for compute_magnetic_field - add tolerances for testing axisymmetric Bnorm (maybe #864)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CoilSet with NFP and sym #864

CoilSet with NFP and sym #864

ddudt commented Feb 7, 2024 •

edited

Loading

github-actions bot commented Feb 7, 2024 •

edited

Loading

codecov bot commented Feb 9, 2024 •

edited

Loading

ddudt Feb 10, 2024

ddudt Feb 10, 2024

ddudt Feb 10, 2024

dpanici commented Feb 14, 2024

CoilSet with NFP and sym #864

CoilSet with NFP and sym #864

Conversation

ddudt commented Feb 7, 2024 • edited Loading

github-actions bot commented Feb 7, 2024 • edited Loading

codecov bot commented Feb 9, 2024 • edited Loading

Codecov Report

ddudt Feb 10, 2024

Choose a reason for hiding this comment

ddudt Feb 10, 2024

Choose a reason for hiding this comment

ddudt Feb 10, 2024

Choose a reason for hiding this comment

dpanici commented Feb 14, 2024

ddudt commented Feb 7, 2024 •

edited

Loading

github-actions bot commented Feb 7, 2024 •

edited

Loading

codecov bot commented Feb 9, 2024 •

edited

Loading