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

Patch: scipy v1.11 #589

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from
Open

Patch: scipy v1.11 #589

wants to merge 9 commits into from

Conversation

mrava87
Copy link
Collaborator

@mrava87 mrava87 commented Jul 2, 2024

This PR is aimed at solving some of the issues arising in our test suite when scipy v1.14 is used. This is mostly related to an (announced) breaking change in the new version for the scipy.sparse.linalg.cg solver, namely:

The tol argument of scipy.sparse.linalg.{bcg,bicstab,cg,cgs,gcrotmk, mres,lgmres,minres,qmr,tfqmr} 
has been removed in favour of rtol. Furthermore, the default value of atol for these 
functions has changed to 0.0.

See https://docs.scipy.org/doc/scipy/release/1.14.0-notes.html

As a result of this change, we decided to:

  • Bump the minimal python requirement to 3.9
  • Bump the minimal scipy requirement to 1.13
  • Change tol into atol when passed to the scipy cg solver in NormalEquationsInversion

@mrava87 mrava87 changed the title Patch scipyv11 Patch: scipy v1.11 Jul 2, 2024
@mrava87 mrava87 requested a review from cako July 2, 2024 20:37
@mrava87
Copy link
Collaborator Author

mrava87 commented Jul 2, 2024

@cako can you take a look and let me know what you think.

This is the first step towards supporting newer versions of scipy and numpy (with numpy v2 as target once all other dependencies do)

@mrava87
Copy link
Collaborator Author

mrava87 commented Jul 10, 2024

Hi @cako, just checking you saw this… it is not a major thing but since effectively it is an inner breaking change (we don’t really trigger it, scipy does but someone may have the same code we had in one of the tests, which will crash…) I wanted a second opining before going ahead :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant