-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cleanup of DOCP struct, more generic version of OCPsolutionFromDOCP #77
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PierreMartinon here is a brief review.
Since I don't know your code really well, I reviewed mainly OCPSolutionFromDOCP_raw
.
function OCPSolutionFromDOCP(docp, docp_solution_ipopt) | ||
|
||
# could pass some status info too (get_status ?) | ||
return OCPSolutionFromDOCP_raw(docp, docp_solution_ipopt.solution, objective=docp_solution_ipopt.objective, constraints_violation=docp_solution_ipopt.primal_feas, iterations=docp_solution_ipopt.iter,multipliers_constraints=docp_solution_ipopt.multipliers, multipliers_LB=docp_solution_ipopt.multipliers_L, multipliers_UB=docp_solution_ipopt.multipliers_U, message=docp_solution_ipopt.solver_specific[:internal_msg]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is roughly what I though.
end | ||
|
||
# still missing: stopping and success info... | ||
function OCPSolutionFromDOCP_raw(docp, solution; objective=nothing, constraints_violation=nothing, iterations=0, multipliers_constraints=nothing, multipliers_LB=nothing, multipliers_UB=nothing, message=nothing) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function OCPSolutionFromDOCP_raw(docp, solution; objective=nothing, constraints_violation=nothing, iterations=0, multipliers_constraints=nothing, multipliers_LB=nothing, multipliers_UB=nothing, message=nothing) | |
function OCPSolutionFromDOCP_raw(docp, solution; objective=DOCP_objective(solution, docp), constraints_violation=nothing, iterations=0, multipliers_constraints=nothing, multipliers_LB=nothing, multipliers_UB=nothing, message=nothing) |
I did not check all optional arguments, but I saw that objective remains nothing
if it is not properly define.
DOCP_objective(get_variable(solution, docp), docp)
should do the trick.
Maybe it can be directly integrated in parse_DOCP_solution()
(but that would require duplication if objective
is already properly set).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, added as well as the constraints violation. Done in the function body since the constraints part is a bit more involved (need to check both variables and constraints bounds)
No description provided.