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

DOC: Add efficiency condition for CSC sparse matrix and remove CSC warning. #15846

Merged
merged 7 commits into from Apr 26, 2022

Conversation

v0dro
Copy link
Contributor

@v0dro v0dro commented Mar 22, 2022

Closes gh-11145. Remove warnings for efficiency when using CSC matrices.

@tupui tupui added enhancement A new feature or improvement scipy.sparse.linalg labels Mar 22, 2022
@perimosocordiae
Copy link
Member

I'd prefer to keep the warning and update the documentation to say that CSC format is preferred/expected (and drop the mention of CSR). We should also reword the warning to say something like:

warn('splu converted its input to CSC format', SparseEfficiencyWarning)

The idea here is that we want users to know when sparse format conversions are happening under the hood, so they can make informed decisions about which format to use for their application code.

Copy link
Member

@perimosocordiae perimosocordiae left a comment

Choose a reason for hiding this comment

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

See my comment #15846 (comment)

Should be sufficient to remove the mention of CSR format in the docstrings, and edit the warning message.

scipy/sparse/linalg/_dsolve/linsolve.py Outdated Show resolved Hide resolved
scipy/sparse/linalg/_dsolve/linsolve.py Outdated Show resolved Hide resolved
@v0dro v0dro marked this pull request as draft April 21, 2022 13:28
…nvey that CSC is the most efficient sparse matrix format.

change warning for splu

DOC: Change documentation of sparse solvers and warnings to better convey that CSC is the most efficient sparse matrix format.

update warning message

update comment

remove brackets
@rgommers
Copy link
Member

@v0dro this looks ready, but is still marked as Draft - any reason to not merge this as is?

@v0dro v0dro marked this pull request as ready for review April 26, 2022 13:10
@rgommers rgommers added this to the 1.9.0 milestone Apr 26, 2022
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks @ v0dro. And thanks @perimosocordiae for the review.

@rgommers rgommers merged commit 0a2fb1a into scipy:main Apr 26, 2022
swallan pushed a commit to swallan/scipy that referenced this pull request Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement scipy.sparse.linalg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unexpected SparseEfficiencyWarning at scipy.sparse.linalg.splu
4 participants