DESC icon indicating copy to clipboard operation
DESC copied to clipboard

Scaled Linear Constraints

Open ddudt opened this issue 1 year ago • 8 comments

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:

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

ddudt avatar Jul 31 '24 23:07 ddudt

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

codecov[bot] avatar Aug 01 '24 00:08 codecov[bot]

|             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  |

github-actions[bot] avatar Aug 01 '24 01:08 github-actions[bot]

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?

f0uriest avatar Aug 01 '24 13:08 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?

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.

ddudt avatar Aug 02 '24 16:08 ddudt

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

dpanici avatar Aug 10 '24 03:08 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

ddudt avatar Aug 12 '24 14:08 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

Can always delete branches, but this is fine

dpanici avatar Aug 12 '24 20:08 dpanici

Use x_scale as optional arg to facotrize linear constraint , if auto use x0 way (if things is not None) otherwise use x_scale as auto

dpanici avatar Aug 14 '24 20:08 dpanici