Update OptimizationFunction, OptimizationProblem and solve docstrings
#431
Conversation
Codecov Report
@@ Coverage Diff @@
## master SciML/Optimization.jl#431 +/- ##
==========================================
+ Coverage 56.47% 56.48% +0.01%
==========================================
Files 54 54
Lines 3938 3937 -1
==========================================
Hits 2224 2224
+ Misses 1714 1713 -1
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
src/problems/basic_problems.jl
Outdated
| * `p`: the constant parameters used for defining the problem. Defaults to `NullParameters`. | ||
| * `lb`: the lower bounds for the optimization variables `u`. | ||
| * `ub`: the upper bounds for the optimization variables `u`. | ||
| * `int`: integrality indicator for `u`. If `int[i] == 1`, then `u[i]` is an integer variable. |
There was a problem hiding this comment.
Integer variables are more general. Integer constraint together with
src/scimlfunctions.jl
Outdated
| - `cons_j(res,x,p)`: the Jacobian of the constraints. | ||
| - `cons_h(res,x,p)`: the Hessian of the constraints, provided as |
There was a problem hiding this comment.
The oop versions are not supported yet right
There was a problem hiding this comment.
No, that's a solver implementation thing, not an interface thing.
There was a problem hiding this comment.
There have been a couple of issues asking about this and since it is not there I think it makes sense to remove it for now and add back once it is implemented?
| - `syms`: the symbol names for the elements of the equation. This should match `u0` in size. For | ||
| example, if `u = [0.0,1.0]` and `syms = [:x, :y]`, this will apply a canonical naming to the | ||
| values, allowing `sol[:x]` in the solution and automatically naming values in plots. | ||
| - `paramsyms`: the symbol names for the parameters of the equation. This should match `p` in | ||
| size. For example, if `p = [0.0, 1.0]` and `paramsyms = [:a, :b]`, this will apply a canonical | ||
| naming to the values, allowing `sol[:a]` in the solution. | ||
| - `hess_colorvec`: a color vector according to the SparseDiffTools.jl definition for the sparsity |
There was a problem hiding this comment.
I have added the fields back but removing it from docstrings seems reasonable until there is a release with support for it?
There was a problem hiding this comment.
These pieces are still correct if they are unused though. It's not like it leads to any correctness, it's just lack of optimization for them to not be used.
There was a problem hiding this comment.
But as a user, it would be confusing to see documented fields that are not used anywhere? It will be trivial to add them back as soon as their use gets implemented
There was a problem hiding this comment.
It will be trivial to add them back as soon as their use gets implemented
We have a policy of not going this direction because it never happens as planned. Docs of interfaces stay complete. If a package isn't matching the interface, the package should change.
But as a user, it would be confusing to see documented fields that are not used anywhere?
We've never had issues with that in the past. Not useful features are generally just ignored until they are useful. And this one is deep in the weeds.
src/problems/basic_problems.jl
Outdated
| for `u₀`; one is allowed to provide `u₀` as arbitrary matrices / | ||
| higher-dimension tensors as well. | ||
| higher-dimension tensors as well if the optimization solver supports it. |
There was a problem hiding this comment.
which optimizers don't support it? why is it not vec/reshaped?
There was a problem hiding this comment.
Just not implemented
src/scimlfunctions.jl
Outdated
| - `cons_j(res,x,p)`: the Jacobian of the constraints. | ||
| - `cons_h(res,x,p)`: the Hessian of the constraints, provided as |
There was a problem hiding this comment.
The oop versions are not supported yet right
| - `syms`: the symbol names for the elements of the equation. This should match `u0` in size. For | ||
| example, if `u = [0.0,1.0]` and `syms = [:x, :y]`, this will apply a canonical naming to the | ||
| values, allowing `sol[:x]` in the solution and automatically naming values in plots. | ||
| - `paramsyms`: the symbol names for the parameters of the equation. This should match `p` in | ||
| size. For example, if `p = [0.0, 1.0]` and `paramsyms = [:a, :b]`, this will apply a canonical | ||
| naming to the values, allowing `sol[:a]` in the solution. | ||
| - `hess_colorvec`: a color vector according to the SparseDiffTools.jl definition for the sparsity |
There was a problem hiding this comment.
I have added the fields back but removing it from docstrings seems reasonable until there is a release with support for it?
src/scimlfunctions.jl
Outdated
| - `hv(Hv,u,v,p)` or `Hv=hv(u,v,p)`: the Hessian-vector product ``(d^2 f / du^2) v``. | ||
| - `cons(res,x,p)` or `cons(x,p)` : the constraints function, should mutate or return a vector | ||
| - `grad(G,u,p)`: the gradient of `f` with respect to `u`. If `f` takes additional arguments | ||
| then `grad(G,u,p,args...)` should be used. |
There was a problem hiding this comment.
? what's a case for that?
Co-authored-by: Qingyu Qu <[email protected]>
Co-authored-by: Christopher Rackauckas <[email protected]>
Vaibhavdixit02
force-pushed
the
optdocs
branch
from
451b260 to
b229033
Compare
src/scimlfunctions.jl
Outdated
| then `grad(G,u,p,args...)` should be used. | ||
| - `hess(H,u,p)`: the Hessian of `f` with respect to `u`. If `f` takes additional arguments | ||
| then `hess(H,u,p,args...)` should be used. | ||
| - `hv(Hv,u,v,p)`: the Hessian-vector product ``(d^2 f / du^2) v``. If `f` takes additional arguments |
There was a problem hiding this comment.
These are missing the out of place definitions.
|
LGTM? |
Fixes SciML/Optimization.jl#498 SciML/OptimizationBase.jl#12