Scaled Linear Constraints

[ddudt]

Resolves #1155

After discussion with @dpanici, we decided the best solution was to add an attribute to the _Coil class to normalize the magnitude of the current. This effectively allows the user to specify the currents in units other than Amps, e.g. MA, so that the raw values are closer to order unity. This makes the optimization work well with linear constraints on the coil currents.

New solution: Adds a diagonal scaling matrix D based on the magnitude of the values of the particular solution. This scales the variables projected/recovered from the linear constraints such that relative changes in the optimization variables x_reduced result in proportional relative changes in the full state vector x_full.

TODO:

• Multiple Z by D where necessary in perturbations, constraint wrappers, etc.
• Add tests.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.47%. Comparing base (e53ce01) to head (ddb63d2).
Report is 1673 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1158   +/-   ##
=======================================
  Coverage   95.46%   95.47%           
=======================================
  Files          87       87           
  Lines       22268    22277    +9     
=======================================
+ Hits        21258    21268   +10     
+ Misses       1010     1009    -1     

Files with missing lines	Coverage Δ
desc/objectives/utils.py	100.00% <100.00%> (ø)
desc/optimize/_constraint_wrappers.py	95.73% <100.00%> (ø)
desc/perturbations.py	88.63% <100.00%> (ø)
desc/vmec.py	92.31% <100.00%> (ø)

... and 1 file with indirect coverage changes

[github-actions]

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_lowres         |     -4.86 +/- 7.14     | -2.71e-02 +/- 3.99e-02 |  5.31e-01 +/- 3.7e-02  |  5.58e-01 +/- 1.6e-02  |
 test_build_transform_fft_midres         |     +2.30 +/- 3.82     | +1.48e-02 +/- 2.45e-02 |  6.58e-01 +/- 2.0e-02  |  6.43e-01 +/- 1.5e-02  |
 test_build_transform_fft_highres        |     +0.99 +/- 3.78     | +9.92e-03 +/- 3.80e-02 |  1.02e+00 +/- 2.8e-02  |  1.01e+00 +/- 2.6e-02  |
 test_equilibrium_init_lowres            |     +0.10 +/- 7.09     | +3.58e-03 +/- 2.67e-01 |  3.77e+00 +/- 1.7e-01  |  3.77e+00 +/- 2.1e-01  |
 test_equilibrium_init_medres            |     +1.01 +/- 6.30     | +4.26e-02 +/- 2.67e-01 |  4.28e+00 +/- 1.9e-01  |  4.23e+00 +/- 1.8e-01  |
 test_equilibrium_init_highres           |     +1.92 +/- 4.20     | +1.08e-01 +/- 2.36e-01 |  5.72e+00 +/- 2.1e-01  |  5.62e+00 +/- 1.1e-01  |
 test_objective_compile_dshape_current   |     -2.58 +/- 1.94     | -1.01e-01 +/- 7.59e-02 |  3.81e+00 +/- 6.8e-02  |  3.91e+00 +/- 3.4e-02  |
 test_objective_compile_atf              |     -0.41 +/- 2.26     | -3.44e-02 +/- 1.88e-01 |  8.31e+00 +/- 1.7e-01  |  8.35e+00 +/- 8.6e-02  |
 test_objective_compute_dshape_current   |     +0.76 +/- 3.21     | +9.50e-06 +/- 4.03e-05 |  1.27e-03 +/- 2.5e-05  |  1.26e-03 +/- 3.2e-05  |
 test_objective_compute_atf              |     +1.17 +/- 5.61     | +5.03e-05 +/- 2.41e-04 |  4.35e-03 +/- 1.2e-04  |  4.30e-03 +/- 2.1e-04  |
 test_objective_jac_dshape_current       |     +0.50 +/- 10.86    | +1.91e-04 +/- 4.13e-03 |  3.82e-02 +/- 2.8e-03  |  3.81e-02 +/- 3.1e-03  |
 test_objective_jac_atf                  |     -1.09 +/- 2.80     | -2.13e-02 +/- 5.47e-02 |  1.93e+00 +/- 3.8e-02  |  1.95e+00 +/- 3.9e-02  |
 test_perturb_1                          |     -3.30 +/- 5.82     | -4.55e-01 +/- 8.04e-01 |  1.34e+01 +/- 2.3e-01  |  1.38e+01 +/- 7.7e-01  |
 test_perturb_2                          |     +1.11 +/- 3.45     | +2.03e-01 +/- 6.35e-01 |  1.86e+01 +/- 4.0e-01  |  1.84e+01 +/- 4.9e-01  |
-test_proximal_jac_atf                   |     +8.47 +/- 1.31     | +6.31e-01 +/- 9.74e-02 |  8.08e+00 +/- 5.5e-02  |  7.44e+00 +/- 8.1e-02  |
 test_proximal_freeb_compute             |     +0.41 +/- 1.16     | +7.41e-04 +/- 2.08e-03 |  1.80e-01 +/- 1.5e-03  |  1.79e-01 +/- 1.4e-03  |
 test_proximal_freeb_jac                 |     -1.19 +/- 1.31     | -8.89e-02 +/- 9.75e-02 |  7.38e+00 +/- 5.1e-02  |  7.47e+00 +/- 8.3e-02  |
 test_solve_fixed_iter                   |     +0.98 +/- 11.71    | +1.77e-01 +/- 2.11e+00 |  1.82e+01 +/- 1.3e+00  |  1.80e+01 +/- 1.6e+00  |

[f0uriest]

with the usual automatic scaling the variable the optimizer sees is x / |f'(x)|
this is basically making a new function g(y=ax) = f(ax) so the optimizer sees y / |g'(y)| = ax / |a f'(x)| = x \ |f'(x)|, so it seems like it wouldn't actually affect the optimization?

[ddudt]

with the usual automatic scaling the variable the optimizer sees is x / |f'(x)| this is basically making a new function g(y=ax) = f(ax) so the optimizer sees y / |g'(y)| = ax / |a f'(x)| = x \ |f'(x)|, so it seems like it wouldn't actually affect the optimization?

See my reply to your comment on the issue. The underlying problem is with how the linear constraints are projected/recovered, not with the optimization itself. So the automatic scaling within the optimizer does not resolve the issue.

[dpanici]

Is it better to just open a new PR rather than having a "revert all commits" commit?
too late now I suppose

[ddudt]

Is it better to just open a new PR rather than having a "revert all commits" commit? too late now I suppose

It was only 3 commits so not too big of a deal. Technically I think this is the more "correct" git approach so we don't have loose ends in the commit history from unused branches

[dpanici]

Is it better to just open a new PR rather than having a "revert all commits" commit? too late now I suppose

It was only 3 commits so not too big of a deal. Technically I think this is the more "correct" git approach so we don't have loose ends in the commit history from unused branches

Can always delete branches, but this is fine

[ddudt]

This test shouldn't have needed to change, since the scales are all 1. I think the bootstrap current optimization is just really sensitive and the extra identity matrices changed the numerics slightly. Is that plausible?