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

Add scaled diagonally dominant matrix constraint. #9472

Merged
merged 4 commits into from Oct 9, 2018

Conversation

hongkai-dai
Copy link
Contributor

@hongkai-dai hongkai-dai commented Sep 19, 2018

@shensquared


This change is Reviewable

Copy link
Contributor Author

@hongkai-dai hongkai-dai left a comment

Choose a reason for hiding this comment

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

+@frankpermenter for feature review please. Thanks!

Reviewable status: all discussions resolved, platform LGTM missing

Copy link
Contributor

@frankpermenter frankpermenter left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 4 files at r1.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee frankpermenter, platform LGTM missing


solvers/test/scaled_diagonally_dominant_matrix_test.cc, line 58 at r1 (raw file):

he product DXD is diagonally dominant with non-negative diagonal entries

This looks like a linear program. If so, then there is a bug in the formulation. (You cannot check membership in the SDD cone by solving a linear program since it is nonpolyhedral.) I think the constraint you mean to check is existence of D for which A=DXD. This is quadratic, not linear, in D.

Copy link
Contributor Author

@hongkai-dai hongkai-dai left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion, LGTM missing from assignee frankpermenter, platform LGTM missing


solvers/test/scaled_diagonally_dominant_matrix_test.cc, line 58 at r1 (raw file):

Previously, frankpermenter wrote…
he product DXD is diagonally dominant with non-negative diagonal entries

This looks like a linear program. If so, then there is a bug in the formulation. (You cannot check membership in the SDD cone by solving a linear program since it is nonpolyhedral.) I think the constraint you mean to check is existence of D for which A=DXD. This is quadratic, not linear, in D.

If we are searching for a matrix X being SDD, then the program is not LP, but SOCP. On the other hand, if X is given, but only need to determine that if the given X is SDD, then we can do this through an LP.

The idea is that if X is SDD, then there exist a diagonal matrix D, such that A = DXD is diagonally dominant. Namely A(i, j) = d(i) * d(j) * X(i, j). The constraint that A being DD is that A(i, i) >= sum_j A(i, j), which is equivalent to d(i)X(i, i) >= sum_j d(j) X(i, j).

This idea is mentioned as remark 6 of Amir Ali's paper. I added it in the documentation.

Copy link
Contributor

@frankpermenter frankpermenter left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 1 unresolved discussion, platform LGTM missing

@jwnimmer-tri
Copy link
Collaborator

+@soonho-tri for platform review per schedule please.

Copy link
Member

@soonho-tri soonho-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

@frankpermenter, could you check the pending issue?

Reviewed 3 of 4 files at r1, 1 of 1 files at r2.
Reviewable status: 4 unresolved discussions, platform LGTM from [soonho-tri], commit history needs curation


solvers/mathematical_program.h, line 2053 at r2 (raw file):

   * represents the absolute value |X(i, j)| ∀ j ≠ i. The diagonal entries
   * Y(i, i) = X(i, i)
   * The users can refer to DSOS and SDSOS Optimization: More Tractable

nit: Please quote the cited article such as

"More Tractable..."

Also, how about having the arxiv link of it: https://arxiv.org/abs/1706.02586 ?

(FYI: the article appears again below)


solvers/mathematical_program.h, line 2082 at r2 (raw file):

   * X.
   * @pre X(i, j) should be a linear expression of decision variables.
   * @return M for i < j, M[i][j] is the 2 x 2 symmetric matrix

I think the @return part is not clear enough. How about the following?

   * @return A vector of vectors of 2 x 2 symmetric matrices M.
   * For i < j, M[i][j] is
   * <pre>
   * [Mⁱʲ(i, i), Mⁱʲ(i, j)]
   * [Mⁱʲ(i, j), Mⁱʲ(j, j)].
   * </pre>
   * Note that M[i][j](0, 1) = Mⁱʲ(i, j) = (X(i, j) + X(j, i)) / 2.
   * For i >= j, M[i][j] is the zero matrix.

Screen Shot 2018-10-08 at 12.07.35 PM.png


solvers/test/scaled_diagonally_dominant_matrix_test.cc, line 62 at r2 (raw file):

  // d, such that the matrix A defined as A(i, j) = d(j) * X(i, j) is diagonally
  // dominant with positive diagonals.
  // This is explained as Remark 6 of DSOS and SDSOS optimization: more

nit: please consider quoting "DSOS and SDSOS ..."

Copy link
Contributor Author

@hongkai-dai hongkai-dai left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 unresolved discussions, platform LGTM from [soonho-tri], commit history needs curation


solvers/mathematical_program.h, line 2053 at r2 (raw file):

Previously, soonho-tri (Soonho Kong) wrote…

nit: Please quote the cited article such as

"More Tractable..."

Also, how about having the arxiv link of it: https://arxiv.org/abs/1706.02586 ?

(FYI: the article appears again below)

Done


solvers/mathematical_program.h, line 2082 at r2 (raw file):

Previously, soonho-tri (Soonho Kong) wrote…

I think the @return part is not clear enough. How about the following?

   * @return A vector of vectors of 2 x 2 symmetric matrices M.
   * For i < j, M[i][j] is
   * <pre>
   * [Mⁱʲ(i, i), Mⁱʲ(i, j)]
   * [Mⁱʲ(i, j), Mⁱʲ(j, j)].
   * </pre>
   * Note that M[i][j](0, 1) = Mⁱʲ(i, j) = (X(i, j) + X(j, i)) / 2.
   * For i >= j, M[i][j] is the zero matrix.

Screen Shot 2018-10-08 at 12.07.35 PM.png

Done. Thanks!


solvers/test/scaled_diagonally_dominant_matrix_test.cc, line 62 at r2 (raw file):

Previously, soonho-tri (Soonho Kong) wrote…

nit: please consider quoting "DSOS and SDSOS ..."

Done.

Copy link
Member

@soonho-tri soonho-tri left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r3.
Reviewable status: 1 unresolved discussion, platform LGTM from [soonho-tri], commit history needs curation

Copy link
Contributor Author

@hongkai-dai hongkai-dai left a comment

Choose a reason for hiding this comment

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

Dismissed @frankpermenter from a discussion.
Reviewable status: all discussions resolved, platform LGTM from [soonho-tri], commit history needs curation

@hongkai-dai hongkai-dai merged commit d891ecd into RobotLocomotion:master Oct 9, 2018
@hongkai-dai hongkai-dai deleted the sdd branch October 10, 2018 00:05
kunimatsu-tri pushed a commit to kunimatsu-tri/drake that referenced this pull request Nov 6, 2018
Add scaled diagonally dominant matrix constraint.
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.

None yet

4 participants