Skip to content

Coil length/curvature losses, normB_axis sampling, custom_loss scaling - #138

Open
rogeriojorge wants to merge 11 commits into
mainfrom
fix/coil-geometry-losses
Open

rogeriojorge wants to merge 11 commits into
mainfrom
fix/coil-geometry-losses

Conversation

@rogeriojorge

Copy link
Copy Markdown
Member

Four objective/loss bugs:

  • loss_coil_length divided by its default max_coil_length=0 (inf/NaN) and was two-sided, (L/Lmax - 1)^2, so coils shorter than the cap were penalized. It is now a one-sided cap, sum(max(L/Lmax - 1, 0)^2), returns a scalar, and raises ValueError when no positive cap is given.
  • loss_coil_curvature returned one value per coil; it now sums to a scalar (the per-coil value is unchanged, so jnp.sum(...) callers are unaffected).
  • normB_axis used linspace(0, 2pi, n) with the endpoint, sampling phi=0 twice and biasing loss_normB_axis_average; now endpoint=False.
  • custom_loss.__mul__ built a new loss without dependencies, so 3 * loss could not be evaluated (starting_dofs raised KeyError); the scaled loss now shares the original's dependencies.

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

  • test_coil_length_loss_is_a_one_sided_cap (circles of radius 1 and 2; only the long coil is penalized; no-cap raises)
  • test_coil_curvature_loss_is_scalar (exact value 0.25^2 * 2pi)
  • test_normB_axis_samples_one_full_period_without_duplicates
  • test_custom_loss_scalar_multiplication_keeps_dependencies

On main: 4 failed. With this branch: all pass.

Overlap: #119 sets the example's max_coil_length to 42, which is compatible with the new check (main's example still passes 0 and would now get an explicit error instead of NaN). Merges cleanly with #119 and #133.

@eduardolneto
eduardolneto self-requested a review October 10, 2026 19:48
@eduardolneto

Copy link
Copy Markdown
Contributor

@rogeriojorge We can take the target scaling of length cost function. Maybe we can add by target as an option for the future. Using augmented lgarangian ethod one can constraint each coild mean curvature to be exactly the target and not only some sum. Dor the sum we can do a separated function?

@rogeriojorge

Copy link
Copy Markdown
Member Author

Went back to the two-sided target sum((L/L0-1)^2): the one-sided cap broke parity with SIMSOPT's CurveLength penalty in examples/comparisons_simsopt/losses.py, and the ValueError broke max_coil_length=0. Now L0<=0 returns 0 (disabled). Still summed to a scalar. Merged main.

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.

2 participants