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: updated preconditioner doc for iterative linear solver #20517

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Afleloup
Copy link

@Afleloup Afleloup commented Apr 18, 2024

Closes #5818

Reference issue

Closes #5818

What does this implement/fix?

The argument M in iterative linear solver
'bicg', 'bicgstab', 'cg', 'cgs' is not standard notation
in comparison to the literature. It is the inverse of the
preconditioner usually denoted M^-1. Their should be a
precondition on M for the preconditioned conjugate gradient.
It is now explicited in the documentation and linked
to relevant references.

Additional information

References
.. [1] TREFETHEN, Lloyd N. et BAU, David. Numerical linear algebra. 
       Society for Industrial and Applied Mathematics, 2022.
.. [2] SAAD, Yousef. Iterative methods for sparse linear systems. 
       Society for Industrial and Applied Mathematics, 2003.
.. [3] "Conjugate Gradient Method, Wikipedia, 
       https://en.wikipedia.org/wiki/Conjugate_gradient_method
.. [4] "Preconditioner", 
       Wikipedia, https://en.wikipedia.org/wiki/Preconditioner
.. [5] KATRUTSA, Alexandr, BOTCHEV, Mike, OVCHINNIKOV, George, et al. 
       How to optimize preconditioners for the conjugate gradient method: 
       a stochastic approach. arXiv preprint arXiv:1806.06045, 2018.
.. [6] CARABA, Elena. Preconditioned conjugate gradient algorithm. 2008.
.. [7] "Biconjugate gradient stabilized method", 
       Wikipedia, https://en.wikipedia.org/wiki/Biconjugate_gradient_stabilized_method
.. [8] "Conjugate gradient squared", Wikipedia,
       https://en.wikipedia.org/wiki/Conjugate_gradient_squared_method
.. [9] "Biconjugate gradient method", Wikipedia, 
       https://en.wikipedia.org/wiki/Biconjugate_gradient_method

@github-actions github-actions bot added scipy.sparse.linalg scipy.sparse Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org labels Apr 18, 2024
@Afleloup Afleloup force-pushed the updated_doc_sparse_linalg_iterative_solver branch from f1a7dfc to 868d336 Compare April 18, 2024 17:09
scipy/sparse/linalg/_isolve/iterative.py Outdated Show resolved Hide resolved
scipy/sparse/linalg/_isolve/iterative.py Outdated Show resolved Hide resolved
scipy/sparse/linalg/_isolve/iterative.py Outdated Show resolved Hide resolved
scipy/sparse/linalg/_isolve/iterative.py Outdated Show resolved Hide resolved
scipy/sparse/linalg/_isolve/iterative.py Outdated Show resolved Hide resolved
@thalassemia
Copy link
Contributor

thalassemia commented Apr 18, 2024

Thanks for the contribution! These are just typo fixes. Someone familiar with the math still needs to review this.

@ilayn
Copy link
Member

ilayn commented Apr 19, 2024

Thank you @Afleloup

I think this is conflating the point here a bit too much. Just because the argument name is M is not meant to be that we find a preconditioner and invert it and then pass here. We can correct the docs saying simply $MA$ should improve the conditioning of the problem as an extra sentence, and that should be enough.

The theoretical treatment is irrelevant here since the operator name can be anything. It's utility is important.

The point of all the inverses attached to M is to emphasize that it kinda sorta inverts A. But in fact it does a more important thing which is, ideally, improves the condition number of the LHS. Hence inverse is a red herring here. The existing docs read correct to me, but can be clarified for the mismatch with the literature.

Note that the preconditioner definition is all over the place, as noted with the wiki page you added as a reference

It is also common to call $T=P^{-1}$ the preconditioner, rather than $\displaystyle P$, since $P$ itself is rarely explicitly available.

@Afleloup
Copy link
Author

Thanks, I will change it this way then. I didn't see the sentence you mentioned in the wikipedia article and thought the literature I had was consistent with the other notation.

@ilayn
Copy link
Member

ilayn commented Apr 19, 2024

No problem, thank you for tackling this issue regardless.

@Afleloup Afleloup force-pushed the updated_doc_sparse_linalg_iterative_solver branch 2 times, most recently from 4bde210 to 9e03ced Compare April 19, 2024 10:06
    linear solver

    The argument M in iterative linear solver
    'bicg', 'bicgstab', 'cg', 'cgs' has different notations
    in the literature. Its usage has been clarified. Their should be a
    precondition on M for the preconditioned conjugate gradient.
    It is now explicited in the documentation and linked
    to relevant references.
    Closes scipy#5818
@Afleloup Afleloup force-pushed the updated_doc_sparse_linalg_iterative_solver branch from 9e03ced to b9125b3 Compare April 19, 2024 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org scipy.sparse.linalg scipy.sparse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify definition of preconditioner for sparse linear system solvers
3 participants