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

Why both SciPy and NumPy? (edit: help wanted to work around an upstream Numpy bug!) #137

Open
someparsa opened this issue Feb 22, 2023 · 3 comments

Comments

@someparsa
Copy link
Contributor

Installing the package in virtual environment, brought to my eyes that the package needs both NumPy and SciPy. To the best of my understanding, NumPy is built on top of the SciPy (or with improvements to each other / or vice versa). BTW as far as I remember, one of the packages is sufficient to have all the functions available. Can't we omit SciPy from our installations? Also, I saw that NumPy is called in the beginning of a few examples. Can't we ease this for the users that when they import anaStruct, they have functionalities of the NumPy available and they don't need a separate import to have its features?

@smith120bh
Copy link
Collaborator

smith120bh commented Feb 23, 2023

... that's a great question. It looks like Scipy was only ever actually used for one single function call to scipy.linalg.eigvals() in solver.py. But Numpy has a numpy.linalg.eigvals() function of its own. Unless there's some nuance in the implementations on Numpy vs Scipy of which I'm unaware, I don't see why we can't just drop Scipy as requirement. I'll PR that change, make sure tests still pass, and check on any notable implementation differences!

EDIT: Ah, that's the difference. The scipy implementation of eigvals() allows you to set an arbitrary right-hand side matrix while the numpy implementation always assumes that the right-hand side matrix is the identity matrix. Anastruct does indeed require a non-identity right-hand side matrix in its eigenvalue solve.

Nevertheless, this is a single function call. Surely there's something we can do to not require all of scipy as a dependency... I'll need to review some linear algebra references for this though!

@smith120bh
Copy link
Collaborator

Okay, so there is supposed to be a viable way to do this in pure numpy, but it's randomly failing, in what appears to be a manifestation of Numpy bug numpy/numpy#20233 . I'd love help from anyone who understands the intricacies of Python / Conda / MKL / OpenBlas, and could figure out a workaround for this bug!

@smith120bh smith120bh changed the title Why both SciPy and NumPy? Why both SciPy and NumPy? (edit: help wanted to work around an upstream Numpy bug!) Feb 24, 2023
@someparsa
Copy link
Contributor Author

someparsa commented Feb 25, 2023

Nice comments @smith120bh, so we have two targets here. Removing the SciPy dependency from our package (#138) and possibly finding a solution to the NumPy bug. I find the first one easier though.

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

No branches or pull requests

2 participants