Adding QUADCOIL integration to DESC as an objective by lankef · Pull Request #2110 · PlasmaControl/DESC

lankef · 2026-03-03T17:14:21Z

This pull request adds two files under /objective, _quadcoil.py and _quadcoil_utils.py. These allows one to call QUADCOIL using a DESC equilibrium and filament coil set as input, and perform quasi-single-stage optimization on the equilibrium and/or the coilset. There are some additions in coils and curves for conversion from simsopt objects. (since quadcoil is written in the simsopt convention). Winding surface can be either fixed or auto-generated, but optimizing the geometry of the winding surface is not yet implemented. The unit test compares quadcoil to desc's built-in REGCOIL.

…also coming soon.

… optimizable_io.py and objective_funs.py to avoid error for static, empty tuple attributes and _objectives with self.coordinates == \'\'

…opy and pasting.

…nversion, but it's not working because quadcoilproxy does not directly store the input dictionary, and desc static dict cannot contain strings (?)

…: Colorbar layout of new layout engine not compatible with old engine, and a colorbar has been created. Engine not changed.

…for merging into main

…e-commit Merging changes from main into the quadcoil branch to prepare for addition into the main branch

github-actions · 2026-03-03T17:39:06Z

Memory benchmark result

|               Test Name                |      %Δ      |    Master (MB)     |      PR (MB)       |    Δ (MB)    |    Time PR (s)     |  Time Master (s)   |
| -------------------------------------- | ------------ | ------------------ | ------------------ | ------------ | ------------------ | ------------------ |
  test_objective_jac_w7x                 |   -1.07 %    |     3.954e+03      |     3.912e+03      |    -42.27    |       39.45        |       37.44        |
  test_proximal_jac_w7x_with_eq_update   |    0.78 %    |     6.635e+03      |     6.687e+03      |    51.93     |       161.98       |       163.60       |
  test_proximal_freeb_jac                |   -0.39 %    |     1.327e+04      |     1.321e+04      |    -51.91    |       85.20        |       86.51        |
  test_proximal_freeb_jac_blocked        |   -1.85 %    |     7.607e+03      |     7.466e+03      |   -141.04    |       75.89        |       74.68        |
  test_proximal_freeb_jac_batched        |    0.76 %    |     7.467e+03      |     7.524e+03      |    56.92     |       74.79        |       74.27        |
  test_proximal_jac_ripple               |    1.60 %    |     3.567e+03      |     3.624e+03      |    56.99     |       68.48        |       66.98        |
  test_proximal_jac_ripple_bounce1d      |   -1.17 %    |     3.566e+03      |     3.524e+03      |    -41.86    |       79.14        |       80.93        |
  test_eq_solve                          |    1.85 %    |     1.993e+03      |     2.030e+03      |    36.78     |       96.32        |       95.31        |

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

f0uriest

Just some initial comments after a first pass, I'll have to understand quadcoil a bit better to give a more detailed review.

Also, you should add quadcoil to devtools/dev-requirements.txt so it will be installed for testing. (and maybe also requirements.txt? Or we can leave it as an optional dependency?)

f0uriest · 2026-03-05T17:09:19Z

        "desc.geometry.core.Surface",
        "desc.magnetic_fields._core.MagneticField",
    ],
+    "desc.magnetic_fields._quadcoil_field.QuadcoilField": [


is this class missing?

Thanks for catching this. It's a WIP class and I've decided to not include it here

removed now

f0uriest · 2026-03-05T17:15:12Z

+    return net_poloidal_current_amperes
+
+
+def _ptolemy_identity_rev_precompute(m_1, n_1):


I think you can use desc.vmec_utils.ptolemy_linear_transform instead of this. It returns a matrix A such that A@x_desc = x_vmec. You can create the matrix in the build method. See the QuasisymmetryBoozer objective for an example

I'm already using ptolemy_linear_transform inside this function. It's just a wrapper to reduce some code reusing.

f0uriest · 2026-03-05T17:20:06Z

+    if "winding_stellsym" in source_kwargs.keys():
+        winding_stellsym = source_kwargs["winding_stellsym"]
+    else:
+        winding_stellsym = sym_default


could be winding_stellsym = source_kwargs.get("winding_stellsym", sym_default)

Fixed. Also replaced a few other occurrences.

f0uriest · 2026-03-05T17:31:39Z

+    def __init__(  # noqa: C901
+        self,
+        eq,
+        quadcoil_kwargs,


is there some sensible default we can give for quadcoil kwargs?

I can make it solve the NESCOIL problem by default but it will require a default value for surface_plasma_distance, too. I feel maybe it's sensible to have the user think about it themselves? I've uploaded example ones in the tutorial notebook too.

f0uriest · 2026-03-05T17:32:39Z

+        name="QUADCOIL Proxy",
+        source_grid=None,
+        # External coils - no external coils by default
+        field=[],


lists are mutable so they generally shouldn't be used as default args since it can lead to some weird behavior. Better to use None or an empty tuple

Fixed to None.

f0uriest · 2026-03-05T17:44:18Z

+            target = 0  # default target value
+        # Uses LSE to smooth non-smooth problems rather than slack variables
+        # by default.
+        if "smoothing" not in quadcoil_kwargs.keys():


can use quadcoil_kwargs.setdefault("smoothing", "approx")

f0uriest · 2026-03-05T17:50:50Z

+        else:
+            self.net_toroidal_current_amperes = 0.0
+        if "plasma_coil_distance" in quadcoil_kwargs.keys():
+            # A sign flip is necessary here!


why sign flip? shouldn't distance always be positive?

Because simsopt surface and DESC surface have different handedness. QUADCOIL uses the simsopt convention so a sign flip is necessary. I've added more comments here.

f0uriest · 2026-03-05T17:56:45Z

+        if self._field:
+            from desc.magnetic_fields import SumMagneticField
+
+            self._constants["sum_field"] = SumMagneticField(self._field)


I think it's better to assume that the user will already give a sum field, rather than a possible list of fields to sum. The general format we've been working with is that field is always assumed to be a single MagneticField object

We silently allow this in BoundaryError, VacuumBoundaryError and QuadraticFlux as a stopgap before we actually make things work correctly with SumMagneticField being passed in directly... this is just further impetus to actually get that working

lankef · 2026-03-06T22:19:16Z

devtools/dev-requirements.txt

It can be an optional dependence. I've just added it to dev-requirements.

dpanici · 2026-03-17T19:05:46Z

        "_use_jit",
    ]
    _static_attrs = [
+        "_static_attrs",  # A bug as of Oct 10 2025


specifically, for some reason one needs static_attrs included in the static_attrs or one will get JAX errors. This seems unintuitive but is empirically what we've found

Should I keep this? I don't need to do anything about this right?

dpanici · 2026-03-17T20:24:51Z

+   notebooks/tutorials/coil_optimization_QUADCOIL.ipynb
   notebooks/tutorials/omnigenity.ipynb
   notebooks/tutorials/bootstrap_current.ipynb
   notebooks/tutorials/coil_stage_two_optimization.ipynb


Also need to add QUADCOIL stuff to the API, I think the file is https://github.com/PlasmaControl/DESC/blob/master/docs/api_objectives.rst?plain=1

add the objectives, I mean

Added!
9faf614

dpanici · 2026-03-17T20:41:46Z

+        targets for each objective terms individually besides using ``target``
+        and ``bounds`` that comes with other DESC objectives.
+        Targets of each property. 0 by default.
+    metric_weight : scalar or ndarray, default=None


1.0 not None

Should be fixed now.

dpanici · 2026-03-17T20:44:13Z

+    return Bnormal
+
+
+# Flipping the current potential of a quadcoil


what are the different coordinate directions/representations/normalizations used? it woudl be good to list these all somewhere, even if just as a comment or in this PR, of the differences in convention btwn DESC and QUADCOIL. Just so we have it clearly down somewhere. Seems phi is flipped?

dpanici · 2026-03-17T20:45:35Z

+    return phi_swapped
+
+
+def _quadcoil_phi_to_desc_phi(phi_mn_quadcoil, stellsym, mpol, ntor):


this is converting just phi_mn quadcoil to desc?

dpanici · 2026-03-17T20:47:03Z

+    return Phi_mn, modes_M, modes_N
+
+
+def _create_source(eq, source_grid, eval_grid):


this could be placed perhaps into the integrals/singularities.py or in a util file there. Also could be used again inside of the other places like BoundaryError. I think it is a reasonable util function, with perhaps some additional documentation attached to it

dpanici · 2026-03-17T20:47:37Z

+    return (source_profiles, source_transforms, interpolator)
+
+
+def _quadcoil_kwargs_to_field_kwargs(  # noqa: C901


would be nice to document what is being done here exactly?

dpanici · 2026-03-17T20:50:26Z

+            plasma_dofs = jnp.concatenate([rc, rs, zc, zs])
+        return plasma_dofs
+
+    def compute_surface_current_field(self, *all_params):


instead of compute_surface_current_field maybe something more like run_regcoil or solve_quadcoil_surface_current_field or something is better?

I renamed this to solve_quadcoil_surface_current_field, I also renamed compute_full to solve_quadcoil.

…ield to solve_quadcoil_surface_current_field

…maControl/DESC into ffu/quadcoil-qss-pre-commit

lankef

Addressed the comments

lankef and others added 30 commits April 4, 2025 12:23

initial commit to add a quadcoil-based objective. A direct interface …

e930a2e

…also coming soon.

initial commit of the QUADCOIL objective, tweaking 2 control flows in…

69a5e87

… optimizable_io.py and objective_funs.py to avoid error for static, empty tuple attributes and _objectives with self.coordinates == \'\'

still debugging instability

74892d8

Improving printing

395d8f9

Initial commit about single-stage

6426650

fixing init

925e19e

Updating QuadcoilObject

773651e

Completing QUADCOIL single-stage interface, testing

c36d534

Renaming and minor bug fixes

dc6a65e

Fixes for static attrs

45e88d8

Fixes for static attrs

f243182

Merge remote-tracking branch 'origin/master' into ffu/quadcoil

59ec216

Merge remote-tracking branch 'origin/master' into ffu/quadcoil

a7c368f

Merge remote-tracking branch 'origin/master' into ffu/quadcoil

87c86e4

Testing for static argument error

7e6d350

Fixing quadcoil

6507199

Adding simsopt curve conversion

c4300f5

Accommodating external fields in QuadcoilProxy

c4754d4

Minor changes

25e96af

QuadcoilField first working. Adding external coil support.

3afd240

Fixing single stage. Re-organizing quadcoil-related codes to reduce c…

e47c443

…opy and pasting.

minor fix, registering quadcoilfield.

8d8de33

Fixing data_index for quadcoilfield

b96e3db

Adding conversion from QuadcoilProxy to QuadcoilField for cutting

18e82c0

Minor update. Attempting to write a quadcoil proxy-> quadcoilfield co…

a5455f8

…nversion, but it's not working because quadcoilproxy does not directly store the input dictionary, and desc static dict cannot contain strings (?)

Minor correction, fixing coil conversion to follow simsopt updates

1d0ff07

Merge remote-tracking branch 'origin/master' into ffu/quadcoil

0ecd9f1

Moving _set_tight_layout from line 988 to 973 to avoid RuntimeError…

b6fea15

…: Colorbar layout of new layout engine not compatible with old engine, and a colorbar has been created. Engine not changed.

removing quadcoil single-stage files that are immature and not ready …

8fa750d

…for merging into main

removing quadcoil single-stage files that are immature and not ready …

7b576d1

…for merging into main

Lanke Fu and others added 5 commits February 24, 2026 10:37

Minor fix, starting documentation

312c961

Merge remote-tracking branch 'origin/master' into ffu/quadcoil-qss-pr…

778f77b

…e-commit Merging changes from main into the quadcoil branch to prepare for addition into the main branch

Adding a QUADCOIL test case. Cleaning up quadcoil integration.

39d5bea

Adding docstrings

06c74ff

Merge branch 'master' into ffu/quadcoil-qss-pre-commit

702d446

lankef requested review from dpanici and f0uriest March 5, 2026 15:27

f0uriest reviewed Mar 5, 2026

View reviewed changes

Lanke Fu added 3 commits March 6, 2026 18:06

Responding to Rory Conlin's comments

1cfc920

Updating docstring

66250f0

Fixing docstring

61c5c12

dpanici reviewed Mar 17, 2026

View reviewed changes

lankef and others added 4 commits March 25, 2026 14:56

Add QuadcoilProxy objective to API documentation

9faf614

Update docstring default values for metric parameters in _quadcoil.py

fcd9cc1

Renaming compute_full to solve_quadcoil and compute_surface_current_f…

cf3ad2b

…ield to solve_quadcoil_surface_current_field

Merge branch 'ffu/quadcoil-qss-pre-commit' of https://github.com/Plas…

13d3ad2

…maControl/DESC into ffu/quadcoil-qss-pre-commit

lankef commented Mar 25, 2026

View reviewed changes

dpanici requested review from dpanici and f0uriest March 26, 2026 17:45

YigitElma and others added 2 commits April 2, 2026 13:28

Merge branch 'master' into ffu/quadcoil-qss-pre-commit

2ea1032

Merge branch 'master' into ffu/quadcoil-qss-pre-commit

7dfc348

		return net_poloidal_current_amperes


		def _ptolemy_identity_rev_precompute(m_1, n_1):

		return Bnormal


		# Flipping the current potential of a quadcoil

		return phi_swapped


		def _quadcoil_phi_to_desc_phi(phi_mn_quadcoil, stellsym, mpol, ntor):

		return Phi_mn, modes_M, modes_N


		def _create_source(eq, source_grid, eval_grid):

		return (source_profiles, source_transforms, interpolator)


		def _quadcoil_kwargs_to_field_kwargs( # noqa: C901

Conversation

lankef commented Mar 3, 2026

Uh oh!

github-actions bot commented Mar 3, 2026

Memory benchmark result

Uh oh!

f0uriest left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

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!

lankef commented Mar 6, 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!

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!

lankef left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects