-
Notifications
You must be signed in to change notification settings - Fork 112
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
change restrt to restart; deprecate restrt #398
Conversation
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## main #398 +/- ##
==========================================
- Coverage 82.39% 82.23% -0.16%
==========================================
Files 83 83
Lines 12454 12479 +25
Branches 1884 1892 +8
==========================================
+ Hits 10261 10262 +1
- Misses 1606 1626 +20
- Partials 587 591 +4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
pyamg/krylov/_fgmres.py
Outdated
@@ -108,6 +110,15 @@ def fgmres(A, b, x0=None, tol=1e-5, | |||
http://www-users.cs.umn.edu/~saad/books.html | |||
|
|||
""" | |||
if restrt is None: | |||
restrt = restart |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be a pass, and not restrt = restart
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch; I think it should be flipped.
pyamg/krylov/_fgmres.py
Outdated
restrt = restart | ||
elif restart is not None: | ||
raise ValueError('Only use restart, not restrt (deprecated).') | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also set restart=restrt
here in the final else?
If so, then we'd probably need to change this also in all the else
statements in the other routines, too
@@ -108,6 +110,14 @@ def fgmres(A, b, x0=None, tol=1e-5, | |||
http://www-users.cs.umn.edu/~saad/books.html | |||
|
|||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think this looks good
restrt
is deprecated in SciPy 1.10.0: https://docs.scipy.org/doc/scipy/release/1.10.0-notes.htmlThis follows the approach in scipy/scipy#16392
Fixes #395