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

Dense graph solvers are unable to accept upper/lower trianglar matrices. #57

Closed
shinmorino opened this issue Oct 9, 2018 · 3 comments
Closed
Labels
bug
Milestone

Comments

@shinmorino
Copy link
Owner

@shinmorino shinmorino commented Oct 9, 2018

All dense graph solvers are able to deal only with symmetric matrices.
symmetrize() has been added to enable upper/lower triangle matrices, there's a misunderstanding on the equation.

Fix:

  1. Check if a given matrix is symmetric.
    1-1. If symmetric, the matrix will be used as is.
  2. If asymmetric, check if the matrix is upper/lower triangular.
    2-1 If triangular, symmetrize the mat.
  3. Error: Raise exception.

This fix will be applied to all dense graph solvers and functions.

@shinmorino shinmorino added the bug label Oct 9, 2018
@shinmorino shinmorino added this to the Version 1.0 milestone Oct 9, 2018
shinmorino added a commit that referenced this issue Oct 9, 2018
shinmorino added a commit that referenced this issue Oct 9, 2018
@shinmorino

This comment has been minimized.

Copy link
Owner Author

@shinmorino shinmorino commented Oct 18, 2018

Fixes are committed at 45d9005.
Tests have been added.
Py and CPU modules have been tested.

@shinmorino shinmorino changed the title Dense graph solvers are unable to accept upper/lower triangle matrices. Dense graph solvers are unable to accept upper/lower trianglar matrices. Oct 18, 2018
@shinmorino

This comment has been minimized.

Copy link
Owner Author

@shinmorino shinmorino commented Oct 18, 2018

This bug is from beta2.
Before beta2, sqaod is assumed to accept only symmetric matrices, and beta2 is also able to handle symmetric matrices.

@shinmorino

This comment has been minimized.

Copy link
Owner Author

@shinmorino shinmorino commented Oct 25, 2018

Done. 7145fca.

@shinmorino shinmorino closed this Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.