-
Notifications
You must be signed in to change notification settings - Fork 16
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
Reduce memory usage in gKDR #244
Conversation
- Possible since the regularized Gram matrix is Hermitian - Use some more descriptive variable names
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.
Looks good. If you merge this into main, please bump the version number and also merge into devel. (If the fix is less urgent feel free to merge into devel instead and we'll make it part of the next release.)
@edaub - thanks for the review - I've now finally resolved your comments above. I've left the target of the PR as main but the version number for you to bump up as discussed. |
I've decided to stick the three current outstanding bugfixes into a single bugfix release 0.7.2, so changing the base branch to put them all onto a single temporary branch prior to merging. I'll also merge this into |
* added bugfix to delete GPU .so file committed to devel to bugfix release on main * Update documentation to reflect patsy requirement (#245) A previous major upgrade added patsy as a requirement rather than an optional dependency. The documentation (readme and installation page on the sphinx docs) now have been updated to reflect this. * Reduce memory usage in gKDR (#244) * Test that a larger gKDR example can run * Reduce memory footprint in gKDR with an explicit loop * Use cholesky/cho_solve instead of general linear solver - Possible since the regularized Gram matrix is Hermitian - Use some more descriptive variable names * Add explanatory docstring * Generate deterministic test data for 'large' gKDR test * Additional docstrings * Use cho_factor instead of cholesky, which simplifies immediate calls to cho_solve Co-authored-by: ots22 <ots22@users.noreply.github.com>
Fixes #242.
The current implementation of gKDR exclusively uses array computations. A drawback of this is that there are some large temporary arrays created (the largest having size
N^2 M^2
whereN
is the number of observations andM
is the number of features).This PR replaces one of these array computations with an explicit loop, reducing the space used to
O(M^2 + M N^2)
.It also replaces two calls to
np.linalg.solve
with calls tocho_solve
(and a single Cholesky decomposition).