Dear developers.
Thanks to developing and maintaining this project!
I think PR #1482 may have introduced a regression in SolverBoxFDDP.
Bug description
SolverBoxFDDP::computePolicy() applies the box-QP only when is_feasible_ == true:
if (!problem_->get_runningModels()[t]->get_has_control_limits() ||
!is_feasible_) {
SolverFDDP::computePolicy(t);
return;
}
This condition also existed before #1482. However, before #1482, SolverFDDP::solve() propagated the is_feasible argument to setCandidate():
setCandidate(init_xs, init_us, is_feasible);
and after accepting a step:
setCandidate(xs_try_, us_try_, (was_feasible_) || (steplength_ == 1));
After #1482, the common implementation in SolverAbstractTpl::solve() appears to ignore the is_feasible argument:
bool SolverAbstractTpl<Scalar>::solve(..., const bool, ...)
{
...
setCandidate(init_xs, init_us, false);
...
if (acceptstep_) {
setCandidate(xs_try_, us_try_, false);
}
}
As a result, even when calling:
solver.solve(xs, us, maxiter, true);
is_feasible_ remains false, and SolverBoxFDDP::computePolicy() falls back to the vanilla FDDP policy instead of solving the box-QP. This seems to make control box constraints ineffective in the backward pass.
A possible fix may be:
setCandidate(init_xs, init_us, is_feasible);
and after an accepted step:
setCandidate(xs_try_, us_try_, is_feasible_ || steplength_ == Scalar(1.));
This would restore the behavior that existed in SolverFDDP::solve() before #1482.
Could you confirm whether this is intentional or a regression?
By the way, there is a following comment line in solver-base.hxx
// TODO: Deprecate isfeasible_. Update setCandidate API.
What is a possible update?
Dear developers.
Thanks to developing and maintaining this project!
I think PR #1482 may have introduced a regression in
SolverBoxFDDP.Bug description
SolverBoxFDDP::computePolicy()applies the box-QP only whenis_feasible_ == true:This condition also existed before #1482. However, before #1482, SolverFDDP::solve() propagated the
is_feasibleargument to setCandidate():setCandidate(init_xs, init_us, is_feasible);and after accepting a step:
After #1482, the common implementation in
SolverAbstractTpl::solve()appears to ignore theis_feasibleargument:As a result, even when calling:
solver.solve(xs, us, maxiter, true);is_feasible_remains false, andSolverBoxFDDP::computePolicy()falls back to the vanilla FDDP policy instead of solving the box-QP. This seems to make control box constraints ineffective in the backward pass.A possible fix may be:
setCandidate(init_xs, init_us, is_feasible);and after an accepted step:
This would restore the behavior that existed in SolverFDDP::solve() before #1482.
Could you confirm whether this is intentional or a regression?
By the way, there is a following comment line in
solver-base.hxx// TODO: Deprecate isfeasible_. Update setCandidate API.What is a possible update?