Skip to content

Switch default solver to scipy.linalg.solve() [#846 issue]#904

Merged
s3alfisc merged 3 commits intopy-econometrics:masterfrom
AronNemeth:switch-default-solver
May 19, 2025
Merged

Switch default solver to scipy.linalg.solve() [#846 issue]#904
s3alfisc merged 3 commits intopy-econometrics:masterfrom
AronNemeth:switch-default-solver

Conversation

@AronNemeth
Copy link
Copy Markdown
Contributor

Related issue #846

Changed default solver to Cholesky decomposition implemented in scipy.linalg.solve(A,b, assume_a="pos").

@s3alfisc
Copy link
Copy Markdown
Member

Thanks @AronNemeth ! In principle this looks good, but we seem to run into an rpy2 error. Sometimes these go away by just waiting a bit, so I will run the tests again tomorrow and see if the error vanishes by itself 😅

@AronNemeth
Copy link
Copy Markdown
Contributor Author

I updated the test matrix to be symmetric and positive definite, so that Cholesky Decomposition doesn't fail

Copy link
Copy Markdown
Member

@s3alfisc s3alfisc left a comment

Choose a reason for hiding this comment

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

Will merge despite failing workflows. Not sure what is going on there & will debug separately. Thank you @AronNemeth!

@s3alfisc s3alfisc merged commit c40b7cc into py-econometrics:master May 19, 2025
4 of 8 checks passed
damandhaliwal pushed a commit to damandhaliwal/pyfixest that referenced this pull request Jun 17, 2025
…sue] (py-econometrics#904)

* Solvers: switching to scipy.linalg.solve as default

* update test/solvers

* Update tests_solvers: new test matrix is symmetric and positive definite -> Cholesky works

---------

Co-authored-by: Alexander Fischer <alexander-fischer1801@t-online.de>
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.

2 participants