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 allocations in tridiagonal LU factorization #648

Closed
simonbyrne opened this issue Jul 14, 2022 · 3 comments · Fixed by #715
Closed

memory allocations in tridiagonal LU factorization #648

simonbyrne opened this issue Jul 14, 2022 · 3 comments · Fixed by #715

Comments

@simonbyrne
Copy link
Member

The call to lu! in the linear solve causes significant memory allocations. My guess is that it is caused by the allocation of the pivot array:
https://github.com/JuliaLang/julia/blob/v1.7.3/stdlib/LinearAlgebra/src/lu.jl#L500

Options:

  • write our own direct tridiagonal solver. @OsKnoth might have some code?
  • use BandedMatrices.jl, which wraps the general LAPACK banded solvers
@simonbyrne
Copy link
Member Author

The best option is probably to implement the Thomas algorithm

@charleskawczynski
Copy link
Member

Is there an issue open in Julia about the allocations? (not that we'll be able to leverage it yet)

@simonbyrne
Copy link
Member Author

no idea: feel free to open one

@simonbyrne simonbyrne mentioned this issue Jul 21, 2022
3 tasks
bors bot added a commit that referenced this issue Jul 28, 2022
715: Adds Thomas algorithm for solving a tri-diagonal system of linear equations. r=sriharshakandala a=sriharshakandala



# PULL REQUEST

## Purpose and Content
Adds Thomas algorithm for solving a tri-diagonal system of linear equations. This replaces the pre-existing `lu!` solver in `linsolve`.

## Benefits and Risks
The Thomas algorithm computes the solution in-place and eliminates memory allocations noticed in the pre-existing `lu!` solver.

## Linked Issues
(Provide references to any link issues. Use closes #issuenum to automatically close an open issue)
- Fixes #648 
- Closes #648 

## PR Checklist
- [x] This PR has a corresponding issue OR is linked to an SDI.
- [x] I have followed CliMA's codebase [contribution](https://clima.github.io/ClimateMachine.jl/latest/Contributing/) and [style](https://clima.github.io/ClimateMachine.jl/latest/DevDocs/CodeStyle/) guidelines OR N/A.
- [x] I have followed CliMA's [documentation policy](https://github.com/CliMA/policies/wiki/Documentation-Policy).
- [x] I have checked all issues and PRs and I certify that this PR does not duplicate an open PR.
- [x] I linted my code on my local machine prior to submission OR N/A.
- [x] Unit tests are included OR N/A.
- [x] Code used in an integration test OR N/A.
- [x] All tests ran successfully on my local machine OR N/A.
- [x] All classes, modules, and function contain docstrings OR N/A.
- [x] Documentation has been added/updated OR N/A.


Co-authored-by: sriharshakandala <[email protected]>
@bors bors bot closed this as completed in 99a8f88 Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants