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

Memory usage reduction #94

Closed
pulquero opened this issue Mar 3, 2021 · 4 comments · Fixed by #95
Closed

Memory usage reduction #94

pulquero opened this issue Mar 3, 2021 · 4 comments · Fixed by #95

Comments

@pulquero
Copy link

pulquero commented Mar 3, 2021

compute_fourier_basis uses self._e, self._U = np.linalg.eigh(self.L.toarray()) this could be replaced with scipy.linalg.eigh(self.L.toarray(), overwrite_a=True) to reuse the memory used by the output of self.L.toarray().

@pulquero
Copy link
Author

pulquero commented Mar 3, 2021

Could also add a spectrum_only param to skip the eigenvectors as they aren't required for some stuff.

@mdeff
Copy link
Collaborator

mdeff commented Mar 18, 2021

Thanks for the suggestions! Reusing the memory is definitely a good idea. Care to make a PR?

I'm less sure about a spectrum_only parameter. In general, adding parameters adds complexity. In most cases, the eigendecomposition is to be avoided altogether. Which cases are you thinking about where we'd want the eigenvalues but not the eigenvectors? Also, the computational cost isn't reduced and the memory must be available for L.toarray() anyway.

@pulquero
Copy link
Author

Btw, I have an implementation of graphwave (about 10 lines), if that would also be of interest.

@mdeff mdeff closed this as completed in #95 Mar 20, 2021
@mdeff
Copy link
Collaborator

mdeff commented Mar 20, 2021

Let's discuss that in #97.

@mdeff mdeff mentioned this issue Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants