Skip to content

Physical coefficients in simsopt/DiscretizedCoils conversions; array PerturbationSample - #139

Merged
rogeriojorge merged 8 commits into
mainfrom
fix/coil-conversion-scaling
Oct 15, 2026
Merged

rogeriojorge merged 8 commits into
mainfrom
fix/coil-conversion-scaling

Conversation

@rogeriojorge

Copy link
Copy Markdown
Member

Curves.dofs (and Coils.dofs_curves) are the optimizer-facing coefficients, _dofs * scaling; the geometry is built from the unscaled _dofs. Three conversions used the scaled values as physical Fourier coefficients, so any coil set with non-default scaling_factor/scale_fixed was exported or converted with the wrong shape:

  • Curves.to_simsopt and Coils.to_simsopt now use _dofs (the coil export also takes dofs_currents_raw directly instead of dofs_currents * currents_scale).
  • DiscretizedCoils.from_Coils now builds the base gamma from coils.curves._dofs. On main, for a radius-2 circle with scaling_factor=0.3, scale_fixed=7, the converted gamma differs from coils.gamma by up to 16.9 m.

Also PerturbationSample.__init__ used if sample: on a JAX array, which raises for any multi-element sample; it is now if sample is not None:.

Tests (fail on main, pass here; JAX_ENABLE_X64=1 pytest -n 2 tests/test_coils.py tests/test_coil_perturbation.py):

  • test_scaled_curves_convert_with_physical_coefficients uses a stub simsopt module so it runs in CI without simsopt, and checks both exports and from_Coils against the physical coefficients and gamma.
  • test_perturbation_sample_accepts_array_sample

On main: 2 failed. With this branch: 37 passed.

Overlap: #132 fixes the related DiscretizedCoils.copy / perturb_curves use of the scaled dofs_gamma and does not touch these lines. This PR does not duplicate it and merges cleanly with #132, #119 and #133.

@rogeriojorge
rogeriojorge merged commit 062b2c9 into main Oct 15, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant