Skip to content

Comments

Add errors, to MagneticWell and MercierStability objectives if grid contains axis.#2098

Open
dpanici wants to merge 7 commits intomasterfrom
dp/stab-error-check
Open

Add errors, to MagneticWell and MercierStability objectives if grid contains axis.#2098
dpanici wants to merge 7 commits intomasterfrom
dp/stab-error-check

Conversation

@dpanici
Copy link
Collaborator

@dpanici dpanici commented Feb 19, 2026

Resolves #2097 by ensuring the situation (which is a bit nonsensical anyways) is not reachable by the user.

Enforces what we already had assumed which was that the user should never pass an on-axis grid point to the MagneticWell or MercierStability objectives (the latter bc it is undefined and should be NaN, the former because it is just zero on-axis anyways by definition of its limit, and also we get NaN in the reverse mode gradient of the on-axis point)

@dpanici dpanici requested review from a team, YigitElma, ddudt, f0uriest, rahulgaur104 and unalmis and removed request for a team February 19, 2026 15:40
@github-actions
Copy link
Contributor

github-actions bot commented Feb 19, 2026

Memory benchmark result

|               Test Name                |      %Δ      |    Master (MB)     |      PR (MB)       |    Δ (MB)    |    Time PR (s)     |  Time Master (s)   |
| -------------------------------------- | ------------ | ------------------ | ------------------ | ------------ | ------------------ | ------------------ |
  test_objective_jac_w7x                 |    6.15 %    |     3.824e+03      |     4.059e+03      |    235.11    |       39.56        |       36.64        |
  test_proximal_jac_w7x_with_eq_update   |    0.13 %    |     6.655e+03      |     6.664e+03      |     8.86     |       165.25       |       162.83       |
  test_proximal_freeb_jac                |   -0.22 %    |     1.323e+04      |     1.320e+04      |    -29.54    |       84.41        |       84.21        |
  test_proximal_freeb_jac_blocked        |   -0.72 %    |     7.559e+03      |     7.505e+03      |    -54.43    |       74.79        |       72.45        |
  test_proximal_freeb_jac_batched        |    0.62 %    |     7.454e+03      |     7.501e+03      |    46.36     |       73.57        |       72.56        |
  test_proximal_jac_ripple               |   -1.35 %    |     3.509e+03      |     3.462e+03      |    -47.24    |       68.07        |       64.45        |
  test_proximal_jac_ripple_bounce1d      |   -1.16 %    |     3.573e+03      |     3.531e+03      |    -41.40    |       76.42        |       76.51        |
  test_eq_solve                          |   -4.15 %    |     2.091e+03      |     2.004e+03      |    -86.77    |       94.73        |       93.91        |

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

@codecov
Copy link

codecov bot commented Feb 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.52%. Comparing base (29c7877) to head (3d32db5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2098      +/-   ##
==========================================
- Coverage   94.52%   94.52%   -0.01%     
==========================================
  Files         102      102              
  Lines       28785    28786       +1     
==========================================
  Hits        27210    27210              
- Misses       1575     1576       +1     
Files with missing lines Coverage Δ
desc/compute/_stability.py 99.17% <100.00%> (ø)
desc/objectives/_stability.py 99.26% <100.00%> (+<0.01%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines 292 to 299
errorif(
grid.axis.size,
ValueError,
"MagneticWell objective grid cannot contain an axis point, "
"as its on-axis limit is always zero."
" Instead, pass axis=False when making the grid, which will "
" automatically place a grid point close but not at rho=0.",
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if we are able to correctly implement the axis limit, since it should be well defined and always 0.

@YigitElma
Copy link
Collaborator

  • replace the division by 0 with safediv

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.

MagneticWell objective could accept on-axis grid point, which does not make sense as it is always zero there

4 participants