Skip to content
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

Feature/hard inequality constraints #35

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

adamheins
Copy link

With reference to #30, this in an in-progress PR for implementing hard inequality constraint support via the HPIPM interface. I've done a basic implementation of general inequality constraints, which seems to work with (1) the friction cone constraints in the legged robot example, and (2) a new test in the hpipm_catkin package with random inequality constraints. At this point I'm just looking for some feedback, in particular with respect to any further testing I can do/add or really just anything I've missed/got wrong/can improve.

The largest remaining task (as mentioned in the original issue), is supporting box inequality constraints directly, since these can be efficiently handled by HPIPM. I've been thinking about this, and it seems to me that we'd need a new type of constraint (say BoxConstraint, presumably specialized to State-only and StateInput like the existing constraints), because the box constraints in HPIPM are (in general) two-sided and I don't believe two-sided constraints are supported directly in OCS2 at the moment. I'm very happy to hear any thoughts on this.

Another item I'm not entirely sure about is handling the inequalityConstraintsSSE for merit/performance index and line search purposes. I've added inequalityConstraintsSSE, which is computed as the sum of squares of the violated inequality constraints (i.e. when the constraint value is negative), but otherwise just treat it in exactly the same way as the existing equalityConstraintsSSE.

wanghg1992 pushed a commit to wanghg1992/ocs2 that referenced this pull request Jun 21, 2022
This commit fixes issue leggedrobotics#35

- CentroidalModelPinocchioMappingTpl::getOcs2Jacobian is called with Jv
  a 0×Nv matrix. This causes a runtime error in line 164 of pinocchio
  mapping.
- Subsequently
  PinocchioEndEffectorKinematics::getPositionLinearApproximation is
  broken when calling getOcs2Jacobian.
- By changing Jv to a zero matrix of size 3×Nv, the runtime error is
  fixed.

// Combine equality and inequality constraints and extract raw data
for (int k = 0; k <= N; k++) {
// Make block diagonal C = [[C_eq, 0 ],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they should be stacked without the zeros, the size of x, the rhs dimension of C should not change:
C = [[C_eq],
[C_inq]]
same for D

I would suggest to load them into the final matrix directly though

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you're right. By loading them into the final matrix directly, do you mean I should avoid having the intermediate C_eq and C_ineq matrices and just build the C_con matrix directly from the constraint objects (same for D)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, we should avoid copying around the data too much. Ideally, if only one of the constraint types is used, we just assign the pointers and don't need a copy at all.

@rubengrandia
Copy link
Contributor

Hi, thanks for this work, and sorry for getting back to you so late.

I think you are on the right track in integrating this. I agree that we should probably introduce a double sided specification for inequalities, and add a specific interface for box constraints as well. I would also suggest to use the slack variable interface of HPIPM, since QP subproblems might be infeasible with the linearized constraints.

On the implementation I think you missed the projection of the inequality contraints. Look for "changeOfInputVariables" in MultipleShootingTranscription.cpp. The change of variables needs to be applied to the linearized inequality contraints, just as for the dynamics.

In terms of testing I suggest to add one test to testCircularKinematics.cpp. This would have revealed the fact that the projection is missing. And a second test for a problem with perfect quadratic cost, linear dynamics, and linear inequality constraints. (the constrained version of testUnconstrained.cpp)

Currently, slacks can be turned on for all constraints or none. Their
penalization and bounds are the same for all slacks and can be set via
settings.
* testCircularKinematics with an inequality constraint (which does force
  the trajectory to change), without and without projection of the
  equality constraint
* testConstrained: analogous to testUnconstrained, which verifies that
  the result of an empty constraint and no constraint at all are the
  same. testConstrained also tests that the constraint is satisfied
  throughout the trajectory.
@adamheins
Copy link
Author

Okay, a number of new changes:

  1. Fixed the construction of the constraint matrices in the HPIPM interface, and also reduced data copying significantly.
  2. New tests in ocs2_sqp, as suggested. In particular, there are new tests in testCircularKinematics.cpp which add an inequality constraint. I've verified that this does in fact change the resulting trajectory. There are tests with and without projection of the equality constraint. In addition, I added a testConstrained.cpp analogous to testUnconstrained.cpp. Like the latter, it first verifies that an empty constraint and null constraint behave the same. Further, it includes a test to verify constraint satisfaction for a random linear inequality constraint.
  3. When creating the new circular kinematics tests, I did run into some feasibility problems so I created a preliminary interface for the slack variables. At the moment information about the slacks is just specified in the HpipmSettings object; I expect we ultimately may wish to handle this differently/in a more full-featured way.

Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants