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

_isolve tacitly checks for symmetric linear operator wrongly #23403

Open
Joshuaalbert opened this issue Sep 3, 2024 · 4 comments
Open

_isolve tacitly checks for symmetric linear operator wrongly #23403

Joshuaalbert opened this issue Sep 3, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@Joshuaalbert
Copy link
Contributor

Description

Instead of taking an explicit symmetric argument, which could be passed from user, or some auxillary information to determine the dtype of linear operator the RHS's dtype is used as a proxy for the dtype of the linear operator when check_symmetric=True. However, it is entirely possible that implicit up-casting is expected by user, such that linear operator is real, but RHS is complex, or that linear operator is symmetric but complex, in both cases matvec == vecmat for positive definite linear operators. This could be a problem for some users of jsp.solve_cg.

# real-valued positive-definite linear operators are symmetric
  def real_valued(x):
    return not issubclass(x.dtype.type, np.complexfloating)
  symmetric = all(map(real_valued, tree_leaves(b))) \
    if check_symmetric else False
  x = lax.custom_linear_solve(
      A, b, solve=isolve_solve, transpose_solve=isolve_solve,
      symmetric=symmetric)

System info (python version, jaxlib version, accelerator, etc.)

jax==0.4.31
jaxlib==0.4.31
@Joshuaalbert Joshuaalbert added the bug Something isn't working label Sep 3, 2024
@jakevdp
Copy link
Collaborator

jakevdp commented Sep 3, 2024

Thanks for the report! So the tricky thing here is that jax.scipy.sparse.linalg.cg implements the API of scipy.sparse.linalg.cg, which has no explicit symmetric flag. We could probably add an optional symmetric argument to JAX's version that lets the user override the default. What do you think?

@Joshuaalbert
Copy link
Contributor Author

That sounds like a good solution.

@jakevdp
Copy link
Collaborator

jakevdp commented Sep 3, 2024

It this a change you're interested in contributing?

@Joshuaalbert
Copy link
Contributor Author

@jakevdp I submitted #23486

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants