Add ShareParameters Linear Objective
#1320
Conversation
| benchmark_name | dt(%) | dt(s) | t_new(s) | t_old(s) |
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
test_build_transform_fft_midres | +0.72 +/- 1.97 | +4.47e-03 +/- 1.22e-02 | 6.25e-01 +/- 8.1e-03 | 6.21e-01 +/- 9.1e-03 |
test_build_transform_fft_highres | -0.12 +/- 1.73 | -1.19e-03 +/- 1.76e-02 | 1.02e+00 +/- 1.1e-02 | 1.02e+00 +/- 1.3e-02 |
test_equilibrium_init_lowres | +4.43 +/- 3.89 | +1.74e-01 +/- 1.53e-01 | 4.11e+00 +/- 1.5e-01 | 3.94e+00 +/- 2.3e-02 |
test_objective_compile_atf | +3.09 +/- 4.55 | +2.43e-01 +/- 3.59e-01 | 8.13e+00 +/- 2.8e-01 | 7.89e+00 +/- 2.2e-01 |
test_objective_compute_atf | +2.98 +/- 1.75 | +3.20e-04 +/- 1.87e-04 | 1.10e-02 +/- 1.5e-04 | 1.07e-02 +/- 1.1e-04 |
test_objective_jac_atf | -0.40 +/- 3.58 | -7.78e-03 +/- 6.98e-02 | 1.94e+00 +/- 5.6e-02 | 1.95e+00 +/- 4.1e-02 |
test_perturb_1 | +4.59 +/- 1.56 | +5.93e-01 +/- 2.02e-01 | 1.35e+01 +/- 1.9e-01 | 1.29e+01 +/- 5.4e-02 |
test_proximal_jac_atf | -0.15 +/- 0.87 | -1.24e-02 +/- 7.08e-02 | 8.15e+00 +/- 6.2e-02 | 8.16e+00 +/- 3.4e-02 |
test_proximal_freeb_compute | -0.36 +/- 1.02 | -6.75e-04 +/- 1.89e-03 | 1.85e-01 +/- 1.0e-03 | 1.85e-01 +/- 1.6e-03 |
test_build_transform_fft_lowres | -4.97 +/- 3.37 | -2.97e-02 +/- 2.01e-02 | 5.68e-01 +/- 1.7e-02 | 5.98e-01 +/- 1.1e-02 |
test_equilibrium_init_medres | -4.92 +/- 2.99 | -2.38e-01 +/- 1.45e-01 | 4.59e+00 +/- 1.2e-01 | 4.83e+00 +/- 8.1e-02 |
test_equilibrium_init_highres | -1.24 +/- 2.00 | -7.48e-02 +/- 1.20e-01 | 5.95e+00 +/- 9.1e-02 | 6.03e+00 +/- 7.9e-02 |
test_objective_compile_dshape_current | -2.18 +/- 1.83 | -9.15e-02 +/- 7.66e-02 | 4.10e+00 +/- 6.1e-02 | 4.19e+00 +/- 4.7e-02 |
test_objective_compute_dshape_current | +6.05 +/- 5.93 | +2.32e-04 +/- 2.28e-04 | 4.07e-03 +/- 1.4e-04 | 3.84e-03 +/- 1.8e-04 |
test_objective_jac_dshape_current | +2.25 +/- 10.51 | +9.79e-04 +/- 4.57e-03 | 4.44e-02 +/- 4.0e-03 | 4.35e-02 +/- 2.1e-03 |
test_perturb_2 | -0.50 +/- 1.92 | -9.83e-02 +/- 3.80e-01 | 1.97e+01 +/- 1.9e-01 | 1.98e+01 +/- 3.3e-01 |
test_proximal_freeb_jac | +2.28 +/- 3.87 | +1.89e-01 +/- 3.21e-01 | 8.48e+00 +/- 2.8e-01 | 8.29e+00 +/- 1.5e-01 |
test_solve_fixed_iter | +0.17 +/- 57.54 | +9.06e-03 +/- 3.07e+00 | 5.34e+00 +/- 2.1e+00 | 5.33e+00 +/- 2.2e+00 | |
dpanici
changed the title
ShareParametersShareParameters Linear Objective
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1320 +/- ##
=======================================
Coverage 95.51% 95.52%
=======================================
Files 96 96
Lines 24044 24067 +23
=======================================
+ Hits 22965 22989 +24
+ Misses 1079 1078 -1
|
| things : list of Optimizable | ||
| list of objects whose degrees of freedom are being fixed to eachother's values. | ||
| Must be at least length 2, but may be of arbitrary length. | ||
| Every object must be of the same type, and have the same size array for the |
There was a problem hiding this comment.
I thought they didn't have to be the same type, just have the same attribute name?
There was a problem hiding this comment.
yea but then I thought of all the ways this could mess up, like Equilibrium.R_lmn and FourierRZToroidalSurface.R_lmn are the same name, yet even if they are the same shape it makes no sense to have them be shared.
I can change it if you think that is not a good enough counterpoint not to, but is there an example where it makes sense to allow this?
| self._params = params | ||
| assert len(things) > 1, "only makes sense for >1 thing" | ||
| assert np.all([isinstance(things[0], type(t)) for t in things[1:]]) | ||
| # TODO: some sort of check that all things are same resolution etc? |
There was a problem hiding this comment.
Could just check that getattr(thing, param).shape is consistent
There was a problem hiding this comment.
Or check the param dict directly
desc/objectives/linear_objectives.py
Outdated
| self._dim_f = sum(idx.size for idx in self._indices) * (len(self.things) - 1) | ||
|
|
||
| # default target | ||
| self.target = np.zeros(self._dim_f) |
There was a problem hiding this comment.
If you set target=0 in init this should happen automatically
There was a problem hiding this comment.
should I make sure that target does not get passed in ever with some sort of check in init? or is not including it in docstring enough to say that one should never pass it in? I worry someone will put a target of like what they wanna fix to and that will mess the objective up, as it would not really be caught anywhere (if the target is say a single number, as it would be broadcastable to the dim_f size)
| self._indices = tree_leaves(self._params) | ||
| assert tree_structure(self._params) == tree_structure(default_params) | ||
|
|
||
| self._dim_f = sum(idx.size for idx in self._indices) * (len(self.things) - 1) |
There was a problem hiding this comment.
Does this account for things where idx is just a book to fix all/none?
There was a problem hiding this comment.
I think so as all of my tests just use True for the param idx.
The default_params and setdefault in this build method are what make sure self._indices becomes the full arange or not if it is True or False
desc/objectives/linear_objectives.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| params_1 : list of dict |
tests/test_linear_objectives.py
Outdated
| J = obj.jac_unscaled(obj.x(eq1, eq2, eq3, eq4)) | ||
| # make sure Jacobian is not trivial | ||
| assert not np.allclose(J, 0) | ||
| # now, check that each row sums to zero, and abs(J) rows sum to 1, |
If someone wants to help with any of below feel free, I just got motivated for the initial implementation.
ShareParameterslinear objective, which takes in a list of arbitrary length of objects and a dict ofparamsto fix whichparamsbeing shared (this check I have not yet implemented)The objective then will fix the indices of the parameters of each object to the same value as each other object.
TODO:
CoilSet(as it currently does not, I don't think, at least not in general. Maybe it does if the coilsets are not nested, I have not tested it for that case)paramslengths being the same across objects (technically, we also should be checking that, say, ifp_lis being fixed btwn two equilibria, that they are also the same TYPE of pressure profile, but not sure how we can go about doing that without having very individualized logic like_check_typedoes incoils.py)Resolves #1250