Changed return of prob.run_driver() from a bool to an object containing information about the Driver execution.
#3214
Conversation
…success" which is the inverse of the previous return value of prob.run_driver. Inidividual driver .run() methods should still return fail, this minimizes breaking backwards compatibility. Drivers may now override _update_results to populate driver results with additional data.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| self.iter_count = 0 | ||
| self.obj_calls = 0 | ||
| self.deriv_calls = 0 | ||
| self.exit_status = 'NOT_RUN' |
There was a problem hiding this comment.
In Aviary and MBSE, we have been using python enums for things like this. It makes it easer for someone coding a new driver, so that they can just import the enum and select one of the exit statuses instead of accidentally making up a new string. (Of course, if they really do have a new kind of status, they can add that to the enum.)
There was a problem hiding this comment.
Hmm, this isn't a new addition though. This is someothing that's alread existent that the optimization report relies upon.
openmdao/core/driver.py
Outdated
| """ | ||
| self.runtime = 0.0 | ||
| self.iter_count = 0 | ||
| self.obj_calls = 0 |
There was a problem hiding this comment.
What do you think about maybe keeping track of runtime for obj_calls and deriv_calls separately? Might be nice to know where the time is being spent.
openmdao/core/driver.py
Outdated
| @@ -87,8 +158,8 @@ class Driver(object): | |||
| Cached total jacobian handling object. | |||
| _total_jac_linear : _TotalJacInfo or None | |||
| Cached linear total jacobian handling object. | |||
| opt_result : dict | |||
| Dictionary containing information for use in the optimization report. | |||
| result : dict | |||
There was a problem hiding this comment.
type is DriverResult vs dict..
…h test_vector_scaled_derivs.
…._run_solve_nonlinear. The latter is a new method.
openmdao/core/driver.py
Outdated
| Parameters | ||
| ---------- | ||
| kind : str | ||
| One of 'obj' or 'deriv', specifying which statistics should be accumulated. |
There was a problem hiding this comment.
maybe pedantic, but i don't know about this being called 'obj', since you don't always have an objective (and, when you do want an objective, you want the constraints too, which also come along for free.) I know that I can't just say this without offering some alternatives ...
"compute" , "compute_totals"
"function", "gradient" (note, this allows for our future "hessian")
"eval", "deriv_eval"
openmdao/core/driver.py
Outdated
| @staticmethod | ||
| def track_stats(kind): | ||
| """ | ||
| Decorate methods to tracks either the objective the deriv time and call count for a Driver. |
There was a problem hiding this comment.
should be 'to track' instead of 'to tracks'
openmdao/core/driver.py
Outdated
| Decorate methods to tracks either the objective the deriv time and call count for a Driver. | ||
|
|
||
| This decorator should be applied to the _objfunc or _gradfunc (or equivalent) methods | ||
| of drivers. It will either accumulated the elapsed time in driver.result.obj_time or |
| @@ -183,7 +183,7 @@ | |||
| "metadata": {}, | |||
| "outputs": [], | |||
| "source": [ | |||
| "fail = prob.run_driver()" | |||
There was a problem hiding this comment.
as future work, we may want to add a __repr__ so that this displays as e.g. a dict in notebooks
openmdao/drivers/doe_driver.py
Outdated
| @@ -73,6 +77,8 @@ def __init__(self, generator=None, **kwargs): | |||
| self._name = '' | |||
| self._problem_comm = None | |||
| self._color = None | |||
| self._num_model_evals = 0 | |||
| self._num_deriv_evals = 0 | |||
There was a problem hiding this comment.
I don't see where these attributes are consumed, they seem redundant with the decorator.. am I missing something?
…and likewise for derivs
| @@ -723,7 +877,7 @@ def get_driver_objective_calls(self): | |||
| int | |||
| Number of objective evaluations made during a driver run. | |||
| """ | |||
| return 0 | |||
| return self.result.model_evals | |||
There was a problem hiding this comment.
The change in terminology makes me wonder if these method names should be deprecated in favor of get_driver_model_evals and get_driver_deriv_evals.
But then I'm not sure why these public methods need to exist... they were only used to populate the opt_result, which is no longer the case. They are not documented anywhere for the user (outside of source docs)...
So maybe these methods should be deprecated entirely in favor of just using the result object.
Also I see that differential_evolution driver overrides the get_objective_calls method, but this doesn't end up affecting the result object now.
There was a problem hiding this comment.
Good point. Changed them to being deprecated and removed their override in DifferentialEvolutionDriver.
Summary
This is the first of several implementation changes in support of POEM 094.
Users were finding the fact that run_driver returned
Trueif something had failed to be confusing.To address this, a new
DriverResultobject has been added, containing the information previously stored indriver.opt_result.This DriverResult object exists for all drivers, including non-optimization ones, so it is now stored in
Driver.result.Previously, ScipyOptimizeDriver stored scipy's OptimizeResult object as
driver.result, but this is now changed toscipy_optimize_result.DOEDriver now successfully populates the attributes of
DriverResult.Hooks have been added, though not yet implemented, to allow drivers to add driver/optimizer-specific attributes to the DriverResults object.
DriverResult currently overrides
__bool__with a deprecation warning to retain existing behavior.Related Issues
Backwards incompatibilities
ScipyOptimizeDriver.resultis now a DriverResults object. Scipy-specific OptimizeResult objects are now stored inScipyOptimizeDriver._scipy_optimize_result, though some of this information will be added to the DriverResult object.New Dependencies
None