Closed Giannis1993 closed 2 years ago
Looks good! Some remarks:
We can pass the plotter as an optional argument too. In that case, the loop
can check for if plotter: plotter.plot(...)
and it does not have to be setup
within this defined loop? If we want a wrapper routine that also plots, my
suggestion would be to include a optimize_and_show
(or something like
that) that does automatically do the plotting.
The imports can be done either in the function or the file that defines this
wrapper. There seem to be not so many that are needed, only Subproblem
,
Bounds
, and MoveLimit
right?
Not sure if we need to use the short hands for some variable names, e.g.
problem
vs prob
, approximation
vs approx
.
We can probably improve this line: ddf = (prob.ddg(x_k) if subprob.approx.__class__.__name__ == 'Taylor2' else None)
to use isinstance(Taylor2)
or in another way such that this check is not needed.
The return arguments for the current solver do not work for all solvers I
think, it might be better to write as x_k, _ = ...
? For this we might need
to unify/clarify the subsolver return arguments.
Implemented Max's feedback and added it to #88. Closing this issue.
This is an issue to discuss the structure of this wrapper function. I think there could be some improvements on the initilializations/imports, but I did not want to use
from somewhere import *
which would make things simpler. My proposal would be to implement something as follows:and would be used as: