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

[PLS Refactor] Linear solver stopping criteria #499

Merged

Conversation

JonathanWenger
Copy link
Contributor

In a Nutshell

This PR adds stopping criteria for the probabilistic linear solver.

Detailed Description

The following stopping criteria are added in addition to an abstract base class:

  • MaxIterationsStopCrit: converge after a maximum number of iterations
  • ResidualNormStopCrit: converge for a sufficiently small residual norm
  • PosteriorContractionStopCrit: converge for sufficiently small trace of the posterior covariance

Related Issues and PRs

PR is part of #51 and supersedes part of #283.

@JonathanWenger JonathanWenger added refactoring Refactoring of existing functionality linalg Issues related to linear algebra labels Jul 22, 2021
@JonathanWenger JonathanWenger added this to the Dagstuhl Workshop milestone Jul 22, 2021
@JonathanWenger JonathanWenger self-assigned this Jul 22, 2021
@codecov
Copy link

codecov bot commented Jul 22, 2021

Codecov Report

Merging #499 (a836666) into main (879cdfe) will increase coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #499      +/-   ##
==========================================
+ Coverage   87.62%   87.72%   +0.10%     
==========================================
  Files         155      160       +5     
  Lines        5963     6012      +49     
  Branches      771      772       +1     
==========================================
+ Hits         5225     5274      +49     
+ Misses        505      504       -1     
- Partials      233      234       +1     
Impacted Files Coverage Δ
.../solvers/information_ops/_linear_solver_info_op.py 100.00% <ø> (ø)
.../probnum/linalg/solvers/information_ops/_matvec.py 100.00% <ø> (ø)
...m/linalg/solvers/information_ops/_proj_residual.py 100.00% <ø> (ø)
...obnum/linalg/solvers/stopping_criteria/__init__.py 100.00% <100.00%> (ø)
...ping_criteria/_linear_solver_stopping_criterion.py 100.00% <100.00%> (ø)
...obnum/linalg/solvers/stopping_criteria/_maxiter.py 100.00% <100.00%> (ø)
...olvers/stopping_criteria/_posterior_contraction.py 100.00% <100.00%> (ø)
...linalg/solvers/stopping_criteria/_residual_norm.py 100.00% <100.00%> (ø)
...um/linalg/solvers/beliefs/_linear_system_belief.py 76.92% <0.00%> (ø)
... and 4 more

@JonathanWenger
Copy link
Contributor Author

Note that this PR is a follow-up to #494 and should therefore be reviewed after.

@JonathanWenger JonathanWenger marked this pull request as ready for review July 22, 2021 22:48
@marvinpfoertner marvinpfoertner self-assigned this Jul 27, 2021
@JonathanWenger JonathanWenger requested review from marvinpfoertner and removed request for marvinpfoertner September 6, 2021 12:49
@marvinpfoertner
Copy link
Collaborator

marvinpfoertner commented Sep 7, 2021

@JonathanWenger I merged main into this branch to facilitate the review. I hope that was fine with you.

Copy link
Collaborator

@marvinpfoertner marvinpfoertner left a comment

Choose a reason for hiding this comment

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

👍🏻

src/probnum/linalg/solvers/stopping_criteria/__init__.py Outdated Show resolved Hide resolved
src/probnum/linalg/solvers/stopping_criteria/_maxiter.py Outdated Show resolved Hide resolved
) -> bool:

residual_norm = np.linalg.norm(solver_state.residual, ord=2)
b_norm = np.linalg.norm(solver_state.problem.b, ord=2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might want to cache this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we previously discussed offline, I would avoid caching the residual or right-hand side at this point, before we actively need it for performance. We may want to either recompute the residual in each step for stability or update the (estimate of) right hand side at each step in the noisy case. Let me know if your opinion on this is different.

@JonathanWenger JonathanWenger merged commit 0dc0878 into probabilistic-numerics:main Sep 17, 2021
@JonathanWenger JonathanWenger deleted the linsolve-stopcrit branch September 17, 2021 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linalg Issues related to linear algebra refactoring Refactoring of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants