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

Reduce memory usage in gKDR #244

Merged
merged 7 commits into from
Nov 4, 2022
Merged

Reduce memory usage in gKDR #244

merged 7 commits into from
Nov 4, 2022

Conversation

ots22
Copy link
Member

@ots22 ots22 commented Sep 5, 2022

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 where N is the number of observations and M 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 to cho_solve (and a single Cholesky decomposition).

@ots22 ots22 requested a review from edaub September 5, 2022 15:54
Copy link
Collaborator

@edaub edaub left a 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.)

mogp_emulator/DimensionReduction.py Outdated Show resolved Hide resolved
@ots22
Copy link
Member Author

ots22 commented Oct 3, 2022

@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.

@edaub edaub changed the base branch from main to v0.7.2rc November 4, 2022 15:13
@edaub
Copy link
Collaborator

edaub commented Nov 4, 2022

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 devel. I don't know if this will fix our issue with main and devel not knowing their common origin, but I'll try and figure this out separately.

@edaub edaub merged commit 5c3c132 into v0.7.2rc Nov 4, 2022
@edaub edaub deleted the gkdr-mem branch November 4, 2022 15:16
edaub added a commit that referenced this pull request Nov 4, 2022
* 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>
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.

Optimization in memory usage of gKDR
2 participants