Add symmetric tridiagonal matrices module#1091
Add symmetric tridiagonal matrices module#1091Mahmood-Sinan wants to merge 23 commits intofortran-lang:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1091 +/- ##
==========================================
- Coverage 68.55% 68.50% -0.06%
==========================================
Files 396 397 +1
Lines 12746 12753 +7
Branches 1376 1376
==========================================
- Hits 8738 8736 -2
- Misses 4008 4017 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
It looks pretty nice. One little thing though (even if I overlooked it when implementing the I'll go through the code a bit more thoroughly tomorrow but it looks pretty good to me already. |
Thanks for the suggestion! You’re right, I implemented so. Now the tridiagonal and symtridiagonal construction is now handled by two pure helper routines(one from arrays and one from constants), which is called by both the pure and impure constructors. |
loiseaujc
left a comment
There was a problem hiding this comment.
I'll take a deeper look at it by the end of the week, but it looks quite good to me so far.
| implicit none | ||
| private | ||
| public :: tridiagonal | ||
| public :: tridiagonal, sym_tridiagonal |
There was a problem hiding this comment.
Modulo this very style question, I think it's pretty much all good for me !
There was a problem hiding this comment.
I also prefer symtridiagonal. However, the style guide says that "Variable and procedure names should be made up of one or more full words separated by an underscore,"
There was a problem hiding this comment.
As @jvdp1 mentioned, if I remember correctly I chose sym_tridiagonal for the same reason
src/specialmatrices/stdlib_specialmatrices_sym_tridiagonal.fypp
Outdated
Show resolved
Hide resolved
src/specialmatrices/stdlib_specialmatrices_sym_tridiagonal.fypp
Outdated
Show resolved
Hide resolved
jvdp1
left a comment
There was a problem hiding this comment.
Thank you @Mahmood-Sinan for this PR. Please find below some comments and suggestions.
| \end{bmatrix}. | ||
| $$ | ||
| Hence, only one vector of size `n` and one vector of size `n-1` need to be stored to fully represent the matrix. | ||
| Interfaces to the most common ones will soon be provided by `stdlib_specialmatrices`. |
There was a problem hiding this comment.
What do you mean be "soon"? Is it not what is proposed in this PR?
There was a problem hiding this comment.
Thank you for noticing. I have missed out a sentence before this which was common for both tridiagonal and symmetric tridiagonal. I added that sentence now. The interfances that this sentence talks about were solve, inverse, etc...
doc/specs/stdlib_specialmatrices.md
Outdated
|
|
||
| `A = ` [[stdlib_specialmatrices(module):sym_tridiagonal(interface)]] `(du, dv)` | ||
|
|
||
| - To construct a symmetric tridiagonal matrix of size `n x n` with constant diagonal elements `du` and `dv`: |
There was a problem hiding this comment.
by "constant" do you mean "scalar"
There was a problem hiding this comment.
Yes, I changed it so scalar now.
| - To construct a symmetric tridiagonal matrix of size `n x n` with constant diagonal elements `du` and `dv`: | ||
|
|
||
| `A = ` [[stdlib_specialmatrices(module):sym_tridiagonal(interface)]] `(du, dv, n)` | ||
|
|
There was a problem hiding this comment.
Please define (type, kind, intent, ...) the variables du, dv, n.
There was a problem hiding this comment.
I have added them now under a new heading named arguments.
| @@ -0,0 +1,17 @@ | |||
| program example_sym_tridiagonal_matrix | |||
| use stdlib_linalg_constants, only: dp | |||
| use stdlib_specialmatrices | |||
There was a problem hiding this comment.
| use stdlib_specialmatrices | |
| use stdlib_specialmatrices, only: sym_tridiagonal_dp_type, sym_tridiagonal |
| implicit none | ||
| private | ||
| public :: tridiagonal | ||
| public :: tridiagonal, sym_tridiagonal |
There was a problem hiding this comment.
I also prefer symtridiagonal. However, the style guide says that "Variable and procedure names should be made up of one or more full words separated by an underscore,"
| ${t1}$, intent(in) :: alpha | ||
| type(sym_tridiagonal_${s1}$_type) :: B | ||
| B = sym_tridiagonal(A%du, A%dv) | ||
| B%du = alpha*B%du; B%dv = alpha*B%dv; |
There was a problem hiding this comment.
| B%du = alpha*B%du; B%dv = alpha*B%dv; | |
| B%du = alpha*B%du | |
| B%dv = alpha*B%dv |
and similarly at other places...
| call random_number(du) ; call random_number(dv) | ||
| A = sym_tridiagonal(du, dv) ; Amat = dense(A) |
There was a problem hiding this comment.
| call random_number(du) ; call random_number(dv) | |
| A = sym_tridiagonal(du, dv) ; Amat = dense(A) | |
| call random_number(du) | |
| call random_number(dv) | |
| A = sym_tridiagonal(du, dv) | |
| Amat = dense(A) |
| allocate(x(n), source = 0.0_wp) ; call random_number(x) | ||
| allocate(y1(n), source = 0.0_wp) ; allocate(y2(n), source=0.0_wp) |
There was a problem hiding this comment.
| allocate(x(n), source = 0.0_wp) ; call random_number(x) | |
| allocate(y1(n), source = 0.0_wp) ; allocate(y2(n), source=0.0_wp) | |
| allocate(x(n)) | |
| call random_number(x) | |
| allocate(y1(n), source = 0.0_wp) | |
| allocate(y2(n), source=0.0_wp) |
| allocate(y1(n), source = 0.0_wp) ; allocate(y2(n), source=0.0_wp) | ||
|
|
||
| ! Test y = A @ x | ||
| y1 = matmul(Amat, x) ; call spmv(A, x, y2) |
There was a problem hiding this comment.
| y1 = matmul(Amat, x) ; call spmv(A, x, y2) | |
| y1 = matmul(Amat, x) | |
| call spmv(A, x, y2) |
|
|
||
| ! Test y = A @ x | ||
| y1 = matmul(Amat, x) ; call spmv(A, x, y2) | ||
| call check(error, all_close(y1, y2), .true.) |
There was a problem hiding this comment.
Add a comment in each check to facilitate the debugging, in case a test is failing (currently it won't be easy to trace back which one is failing)
Thank you so much for reviewing. I have made those changes. Please take a look at them. |
|
@jvdp1 Please handle. |
This draft PR adds support for symmetric tridiagonal matrices inside
specialmatricesin stdlib.It includes implementation, tests, documentation and examples.
The PR implements the same arithmetic and matrix operations as the existing tridiagonal module.
Remaining tasks:
(this will be added here once that implementation is merged).
Feedback is welcome.