Coil_pertubation fix by adding Coils_from_gamma by eduardolneto · Pull Request #30 · uwplasma/ESSOS

eduardolneto · 2026-03-03T21:40:10Z

Adding Coils_from_gamma in coils.py and modifying coil_pertubation.py to work with it. Also modifying simple_examples/create_pertubed_coils.py to work with the changes.

… to work with it. Also modifying simple_examples/create_pertubed_coils.py to work with the changes.

Copilot

Pull request overview

This PR introduces a new coil container that stores discretized coil geometry (gamma) directly (instead of Fourier dofs), and refactors coil perturbation utilities + an example script to work with this representation.

Changes:

Add Coils_from_gammas to represent coils using discretized gamma + numerically computed derivatives.
Refactor perturb_curves_systematic / perturb_curves_statistic to support Coils_from_gammas (and update how Curves/Coils are perturbed via refitting).
Update examples/simple_examples/create_perturbed_coils.py to use Coils_from_gammas.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
examples/simple_examples/create_perturbed_coils.py	Switches the example to build initial/systematic/statistical coils via `Coils_from_gammas`.
essos/coils.py	Adds the `Coils_from_gammas` class (gamma-based coil storage, symmetry expansion, finite-diff derivatives, I/O, conversions).
essos/coil_perturbation.py	Updates perturbation functions to handle `Coils_from_gammas` and refits Fourier dofs for `Curves`/`Coils`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

essos/coil_perturbation.py

Copilot · 2026-03-03T21:45:51Z

essos/coil_perturbation.py

+        dofs_new, _ = fit_dofs_from_coils(gamma_perturbed, curves.order, curves.n_segments)
+        curves.curves = Curves(dofs_new, curves.n_segments, nfp=1, stellsym=False)
+        curves.dofs_currents_raw = curves.currents


In the Coils branch, dofs_currents_raw is set from curves.currents after replacing curves.curves with a Curves(..., nfp=1, stellsym=False). At that point curves.currents no longer returns the expanded current array, so the number of base currents can become inconsistent with the new number of curves. Capture the expanded currents before changing the symmetry/curves object, and then set dofs_currents_raw to that captured array (length should match gamma_perturbed.shape[0]).

Suggested change

dofs_new, _ = fit_dofs_from_coils(gamma_perturbed, curves.order, curves.n_segments)

curves.curves = Curves(dofs_new, curves.n_segments, nfp=1, stellsym=False)

curves.dofs_currents_raw = curves.currents

# Capture the expanded currents before modifying the symmetry/curves object.

expanded_currents = curves.currents

dofs_new, _ = fit_dofs_from_coils(gamma_perturbed, curves.order, curves.n_segments)

curves.curves = Curves(dofs_new, curves.n_segments, nfp=1, stellsym=False)

curves.dofs_currents_raw = expanded_currents

Copilot · 2026-03-03T21:45:51Z

essos/coil_perturbation.py

+        dofs_new, _ = fit_dofs_from_coils(perturbed_base_gamma, curves.order, curves.n_segments)
+        curves.dofs = dofs_new
+        return
+
+    raise TypeError(f"Unsupported type {type(curves)}. Expected Curves, Coils, or Coils_from_gammas.")


perturb_curves_systematic/perturb_curves_statistic now only accept Curves, Coils, or Coils_from_gammas and raise TypeError otherwise. The test suite currently uses a duck-typed DummyCurves in tests/test_coil_perturbation.py, which will now fail. Either broaden support (e.g., accept any object with the required attributes like gamma/dofs/nfp/stellsym) or update the tests/callers accordingly.

Copilot · 2026-03-03T21:45:52Z

essos/coil_perturbation.py

+        curves._nfp = 1
+        curves._stellsym = False


perturb_curves_statistic mutates Coils_from_gammas by directly assigning to private fields (_nfp, _stellsym). This bypasses any cache/invariant management and makes the side effects less discoverable. Prefer adding a small public method on Coils_from_gammas to “drop symmetries” (or expose setters) so the cache reset and invariants are handled in one place.

Suggested change

curves._nfp = 1

curves._stellsym = False

curves.nfp = 1

curves.stellsym = False

Copilot · 2026-03-03T21:45:52Z

essos/coils.py

+class Coils_from_gammas:
+    """ Class to store coils from gamma (discretized curve coordinates) instead of Fourier coefficients
+
+    This class is compatible with the Coils class but stores dofs as the actual gamma values
+    rather than Fourier expansion coefficients. Derivatives are computed numerically.


Class names in this module are consistently CamelCase (Curves, Coils). Coils_from_gammas introduces an underscore-separated class name, which is inconsistent with the existing public API and makes imports/usage less predictable. Consider renaming to a CamelCase variant (e.g., CoilsFromGammas / CoilsFromGamma) and, if needed, keep Coils_from_gammas as a backwards-compatible alias.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

essos/coil_perturbation.py

Copilot · 2026-03-05T20:30:32Z

essos/coil_perturbation.py

+        base_gamma = Curves(curves.dofs_curves, curves.n_segments, nfp=1, stellsym=False).gamma
+        perturbed_base_gamma = base_gamma + perturbation[:, 0, :, :]
+        dofs_new, _ = fit_dofs_from_coils(perturbed_base_gamma, curves.order, curves.n_segments)
+        curves.dofs_curves = dofs_new


In both systematic/statistical paths for Curves/Coils, fit_dofs_from_coils is called with the default assume_uniform=False, which triggers arclength resampling. Here the data comes from uniform quadpoints (and you’re perturbing those samples), so this can likely use assume_uniform=True to avoid an expensive resampling step and speed up perturbations significantly.

This seems important

Copilot · 2026-03-05T20:30:32Z

essos/coils.py

+        """Save coils to JSON file"""
+        data = {
+            "n_segments": self.n_segments,
+            "nfp": self.nfp,
+            "stellsym": self.stellsym,
+            "dofs_gamma": self.dofs_gamma.tolist(),
+            "dofs_currents": self.dofs_currents.tolist(),
+        }


to_json() persists dofs_currents (normalized currents), but from_json() reads them back as the constructor currents (treated as raw currents), which changes the physical current magnitudes on round-trip (unless currents_scale == 1). Store either dofs_currents_raw instead, or also store currents_scale and reconstruct raw currents during load.

This also seems important

Copilot · 2026-03-05T20:30:32Z

essos/coils.py

+
+    @property
+    def curvature(self):
+        return vmap(self.compute_curvature)(self.gamma_dash, self.gamma_dashdash)


CoilsFromGamma defines self._curvature and clears it in reset_cache(), but the curvature property never populates/uses the cache. This recomputes curvature on every access (e.g. repeated plotting/optimization). Consider mirroring the Curves.curvature pattern: compute once, store in self._curvature, and return the cached value until invalidated.

Suggested change

return vmap(self.compute_curvature)(self.gamma_dash, self.gamma_dashdash)

if self._curvature is None:

self._curvature = vmap(self.compute_curvature)(self.gamma_dash, self.gamma_dashdash)

return self._curvature

This is important to mimic the functionality that @EstevaoMGomes introduced

Copilot · 2026-03-05T20:30:32Z

examples/simple_examples/create_perturbed_coils.py

-# Add statistical error
-coils_stat = Coils(curves=curves, currents=[current_on_each_coil]*number_coils_per_half_field_period)
-perturb_curves_statistic(coils_stat, g, key=split_keys[1])
+# Add statistical error (returns a new object because now there are no symmtries in the perturbed coils so nfp and stellsym changes)


Typo in comment: 'symmtries' should be 'symmetries'.

Suggested change

# Add statistical error (returns a new object because now there are no symmtries in the perturbed coils so nfp and stellsym changes)

# Add statistical error (returns a new object because now there are no symmetries in the perturbed coils so nfp and stellsym changes)

rogeriojorge

Mostly good with me, see the few specific comments I wrote above

Adding Coils_from_gamma in coils.py and modifying coil_pertubation.py…

fda3128

… to work with it. Also modifying simple_examples/create_pertubed_coils.py to work with the changes.

eduardolneto requested review from EstevaoMGomes and Copilot March 3, 2026 21:40

Copilot started reviewing on behalf of eduardolneto March 3, 2026 21:40 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

eduardolneto requested a review from rogeriojorge March 3, 2026 21:46

Some extra changes for coil_perturbation

6562157

rogeriojorge requested a review from Copilot March 5, 2026 20:27

Copilot AI reviewed Mar 5, 2026

View reviewed changes

rogeriojorge reviewed Mar 5, 2026

View reviewed changes

-        return vmap(self.compute_curvature)(self.gamma_dash, self.gamma_dashdash)
+        if self._curvature is None:
+            self._curvature = vmap(self.compute_curvature)(self.gamma_dash, self.gamma_dashdash)
+        return self._curvature

	# Add statistical error (returns a new object because now there are no symmtries in the perturbed coils so nfp and stellsym changes)
	# Add statistical error (returns a new object because now there are no symmetries in the perturbed coils so nfp and stellsym changes)

Conversation

eduardolneto commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rogeriojorge Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rogeriojorge Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rogeriojorge Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rogeriojorge Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

rogeriojorge left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants