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

Refactor linear algebra module and remove unused code #195

Open
wants to merge 41 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
6f4fe35
Refactor linear algebra module and remove unused code
ulises-jeremias Jan 7, 2024
d667c95
Update installation instructions for LAPACK-OpenBLAS
ulises-jeremias Jan 7, 2024
10eea19
Update imports and function signatures in lapack and blas modules
ulises-jeremias Jan 7, 2024
5bd805c
Update import statements in lapack module
ulises-jeremias Jan 7, 2024
f33a31b
Update LAPACKE backend status in README.md
ulises-jeremias Jan 7, 2024
3a0af32
Refactor LAPACKE backend for improved performance
ulises-jeremias Jan 7, 2024
11ed075
Refactor README files for BLAS and LAPACK backends
ulises-jeremias Jan 7, 2024
c3d1e2e
Refactor conversion functions to be public
ulises-jeremias Jan 7, 2024
ea376ac
Merge branch 'main' of github.com:vlang/vsl into feature/new-lapack
ulises-jeremias Feb 4, 2024
c7356d7
Merge branch 'main' of github.com:vlang/vsl into feature/new-lapack
ulises-jeremias Feb 25, 2024
0832f6d
Refactor dgetrf function to use blocked algorithm
ulises-jeremias Mar 24, 2024
ae0628b
Merge branch 'main' of github.com:vlang/vsl into feature/new-lapack
ulises-jeremias Mar 24, 2024
2a2e7d8
Merge branch 'main' of github.com:vlang/vsl into feature/new-lapack
ulises-jeremias Apr 1, 2024
e82bda8
Merge branch 'main' of github.com:vlang/vsl into feature/new-lapack
ulises-jeremias Apr 27, 2024
88776da
Refactor dgetrf function to use blocked algorithm
ulises-jeremias Apr 28, 2024
b84f2ba
Refactor dgetrf function to use blocked algorithm and fix variable na…
ulises-jeremias Apr 28, 2024
ab34e62
Refactor LAPACK functions to use row-major memory layout
ulises-jeremias Apr 28, 2024
14d3f67
Refactor LAPACK module to use lapack64 module
ulises-jeremias Apr 28, 2024
02da167
Refactor dgetrs function to use f64 instead of float64 for array types
ulises-jeremias Apr 28, 2024
b2f481c
Merge branch 'main' of github.com:vlang/vsl into feature/new-lapack
ulises-jeremias Jun 17, 2024
e43bfa5
refactor: Update create_image_2d function to use local variable for f…
ulises-jeremias Jun 18, 2024
bfa6907
refactor: Replace constant lookup with a list of constants in ilaenv.v
ulises-jeremias Jun 18, 2024
961475e
refactor: Update create_image_2d function to use local variable for f…
ulises-jeremias Jun 18, 2024
f6cb782
refactor: Update execute tests step in ci.yml to use Pure C Backend w…
ulises-jeremias Jun 18, 2024
569a96b
refactor: Comment out test execution step in ci.yml
ulises-jeremias Jun 18, 2024
d8a4fc2
refactor: Update create_image_2d function to use local variable for f…
ulises-jeremias Jun 18, 2024
c66401f
refactor: Update gemv_test.v to use named arguments in dgemvcomp calls
ulises-jeremias Jun 18, 2024
d399e1a
refactor: Update gemv_test.v to use named arguments in dgemvcomp calls
ulises-jeremias Jun 18, 2024
1d5e441
refactor: Update gemv_test.v to use named arguments in dgemvcomp calls
ulises-jeremias Jun 18, 2024
9f46519
refactor: Update conversions.v, dgetf2.v, dsyev.v, and lapack_notd_vs…
ulises-jeremias Jun 22, 2024
d376d36
refactor: Update dlansy.v to use named constants for error messages
ulises-jeremias Jun 22, 2024
99a3a2b
refactor: Update dpotrf function in lapack_notd_vsl_lapack_lapacke.v …
ulises-jeremias Jun 22, 2024
5f9a1fe
refactor: Update conversions.v, dgetf2.v, dsyev.v, and lapack_notd_vs…
ulises-jeremias Jun 23, 2024
e83de29
refactor: Update LAPACK functions in lapack_notd_vsl_lapack_lapacke.v…
ulises-jeremias Jun 23, 2024
b06e436
refactor: Update ci.yml to execute tests using Pure V Backend
ulises-jeremias Jun 23, 2024
36ae80e
refactor: Update ci.yml to execute tests using Pure V Backend with CB…
ulises-jeremias Jun 23, 2024
63adcf6
refactor: Update ci.yml to execute tests using Pure V Backend with CB…
ulises-jeremias Jun 23, 2024
8afd14a
refactor: Update dpotrf function to use named constant for uplo param…
ulises-jeremias Jun 23, 2024
3af89bf
refactor: Update BLAS and LAPACK functions to use named constants and…
ulises-jeremias Jun 23, 2024
d9fd254
Merge branch 'main' of github.com:vlang/vsl into feature/new-lapack
ulises-jeremias Aug 3, 2024
79d8a8d
Merge branch 'main' of github.com:vlang/vsl into feature/new-lapack
ulises-jeremias Aug 5, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ jobs:
run: mv ./vsl ~/.vmodules

- name: Execute Tests using Pure V Backend
run: ~/.vmodules/vsl/bin/test ${{ matrix.flags }}

- name: Execute Tests using Pure V Backend with Pure C Blas
run: ~/.vmodules/vsl/bin/test ${{ matrix.flags }} --use-cblas

- name: Execute Tests using Pure V Backend with Pure C Backend
run: ~/.vmodules/vsl/bin/test ${{ matrix.flags }} --use-cblas --use-lapacke

run-tests-on-macos:
runs-on: ${{ matrix.os }}

Expand Down Expand Up @@ -110,8 +110,8 @@ jobs:
- name: Move VSL source code to V Modules
run: mv ./vsl ~/.vmodules

- name: Execute Tests using Pure V Backend
run: ~/.vmodules/vsl/bin/test
# - name: Execute Tests using Pure V Backend
# run: ~/.vmodules/vsl/bin/test
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will revert this as soon as I finish testing


- name: Execute Tests using Pure V Backend with Pure C Blas
run: ~/.vmodules/vsl/bin/test --use-cblas
- name: Execute Tests using Pure V Backend with Pure C Backend
run: ~/.vmodules/vsl/bin/test --use-cblas --use-lapacke
8 changes: 7 additions & 1 deletion bin/test
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
## --stats Execute with stats
## --prod Execute with prod build
## --use-cblas Execute tests using cblas
## --use-lapacke Execute tests using lapacke
## --use-autofree Execute tests using atofree
## --use-gc=STRATEGY Execute tests using garbage collector
## --skip-examples Skip examples compilation
Expand All @@ -28,7 +29,12 @@ flags=""

if [[ -n "${use_cblas}" ]]; then
echo "Running tests using Open BLAS"
flags="${flags} -d vsl_vlas_cblas"
flags="${flags} -d vsl_blas_cblas"
fi

if [[ -n "${use_lapacke}" ]]; then
echo "Running tests using LAPACKE"
flags="${flags} -d vsl_lapack_lapacke"
fi

if [[ -n "${use_autofree}" ]]; then
Expand Down
61 changes: 61 additions & 0 deletions blas/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# The V Basic Linear Algebra System

This package implements Basic Linear Algebra System (BLAS) routines in V.

| Backend | Description | Status | Compilation Flags |
| -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ------------------- |
| BLAS | Pure V implementation | Stable | `NONE` |
| OpenBLAS | OpenBLAS is an optimized BLAS library based on <https://github.com/xianyi/OpenBLAS>. Check the section [OpenBLAS Backend](#openblas-backend) for more information. | Stable | `-d vsl_blas_cblas` |

Therefore, its routines are a little more _lower level_ than the ones in the package `vsl.la`.

## OpenBLAS Backend

We provide a backend for the OpenBLAS library. This backend is probably
the fastest one for all platforms
but it requires the installation of the OpenBLAS library.

Use the compilation flag `-d vsl_blas_cblas` to use the OpenBLAS backend
instead of the pure V implementation
and make sure that the OpenBLAS library is installed in your system.

Check the section below for more information about installing the OpenBLAS library.

<details>
<summary>Install dependencies</summary>

### Homebrew (macOS)

```sh
brew install openblas
```

### Debian/Ubuntu GNU Linux

`libopenblas-dev` is not needed when using the pure V backend.

```sh
sudo apt-get install -y --no-install-recommends \
gcc \
gfortran \
libopenblas-dev
```

### Arch Linux/Manjaro GNU Linux

The best way of installing OpenBlas is using
[lapack-openblas](https://aur.archlinux.org/packages/lapack-openblas/).

```sh
yay -S lapack-openblas
```

or

```sh
git clone https://aur.archlinux.org/lapack-openblas.git /tmp/lapack-openblas
cd /tmp/lapack-openblas
makepkg -si
```

</details>
34 changes: 34 additions & 0 deletions blas/blas64/conversions.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
module blas64

// MemoryLayout is used to specify the memory layout of a matrix.
pub enum MemoryLayout {
row_major = 101
col_major = 102
}

// Transpose is used to specify the transposition of a matrix.
pub enum Transpose {
no_trans = 111
trans = 112
conj_trans = 113
conj_no_trans = 114
}

// Uplo is used to specify whether the upper or lower triangle of a matrix is
pub enum Uplo {
upper = 121
lower = 122
all = 99
}

// Diagonal is used to specify whether the diagonal of a matrix is unit or non-unit.
pub enum Diagonal {
non_unit = 131
unit = 132
}

// Side is used to specify whether a matrix is on the left or right side in a matrix-matrix multiplication.
pub enum Side {
left = 141
right = 142
}
23 changes: 12 additions & 11 deletions vlas/internal/blas/dgemm.v → blas/blas64/dgemm.v
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module blas
module blas64

// import runtime
import sync
Expand Down Expand Up @@ -99,10 +99,11 @@ pub fn dgemm(trans_a Transpose, trans_b Transpose, m int, n int, k int, alpha f6
}
}

dgemm_parallel(a_trans, b_trans, m, n, k, a, lda, b, ldb, mut c, ldc, alpha)
dgemm_parallel(if a_trans { .trans } else { .no_trans }, if b_trans { .trans } else { .no_trans },
m, n, k, a, lda, b, ldb, mut c, ldc, alpha)
}

fn dgemm_parallel(a_trans bool, b_trans bool, m int, n int, k int, a []f64, lda int, b []f64, ldb int, mut c []f64, ldc int, alpha f64) {
fn dgemm_parallel(a_trans Transpose, b_trans Transpose, m int, n int, k int, a []f64, lda int, b []f64, ldb int, mut c []f64, ldc int, alpha f64) {
// dgemm_parallel computes a parallel matrix multiplication by partitioning
// a and b into sub-blocks, and updating c with the multiplication of the sub-block
// In all cases,
Expand Down Expand Up @@ -155,7 +156,7 @@ fn dgemm_parallel(a_trans bool, b_trans bool, m int, n int, k int, a []f64, lda
for i := 0; i < m; i += block_size {
for j := 0; j < n; j += block_size {
// worker_limit <- 0
go fn (a_trans bool, b_trans bool, m int, n int, max_k_len int, a []f64, lda int, b []f64, ldb int, mut c []f64, ldc int, alpha f64, i int, j int, mut wg sync.WaitGroup) {
go fn (a_trans Transpose, b_trans Transpose, m int, n int, max_k_len int, a []f64, lda int, b []f64, ldb int, mut c []f64, ldc int, alpha f64, i int, j int, mut wg sync.WaitGroup) {
defer {
wg.done()
// <-worker_limit
Expand All @@ -180,12 +181,12 @@ fn dgemm_parallel(a_trans bool, b_trans bool, m int, n int, k int, a []f64, lda
}
mut a_sub := []f64{}
mut b_sub := []f64{}
if a_trans {
if a_trans == .trans {
a_sub = slice_view_f64(a, lda, k, i, lenk, leni)
} else {
a_sub = slice_view_f64(a, lda, i, k, leni, lenk)
}
if b_trans {
if b_trans == .trans {
b_sub = slice_view_f64(b, ldb, j, k, lenj, lenk)
} else {
b_sub = slice_view_f64(b, ldb, k, j, lenk, lenj)
Expand All @@ -200,20 +201,20 @@ fn dgemm_parallel(a_trans bool, b_trans bool, m int, n int, k int, a []f64, lda
}

// dgemm_serial is serial matrix multiply
fn dgemm_serial(a_trans bool, b_trans bool, m int, n int, k int, a []f64, lda int, b []f64, ldb int, mut c []f64, ldc int, alpha f64) {
if !a_trans && !b_trans {
fn dgemm_serial(a_trans Transpose, b_trans Transpose, m int, n int, k int, a []f64, lda int, b []f64, ldb int, mut c []f64, ldc int, alpha f64) {
if a_trans != .trans && b_trans != .trans {
dgemm_serial_not_not(m, n, k, a, lda, b, ldb, mut c, ldc, alpha)
return
}
if a_trans && !b_trans {
if a_trans == .trans && b_trans != .trans {
dgemm_serial_trans_not(m, n, k, a, lda, b, ldb, mut c, ldc, alpha)
return
}
if !a_trans && b_trans {
if a_trans != .trans && b_trans == .trans {
dgemm_serial_not_trans(m, n, k, a, lda, b, ldb, mut c, ldc, alpha)
return
}
if a_trans && b_trans {
if a_trans == .trans && b_trans == .trans {
dgemm_serial_trans_trans(m, n, k, a, lda, b, ldb, mut c, ldc, alpha)
return
}
Expand Down
2 changes: 1 addition & 1 deletion vlas/internal/blas/dgemv.v → blas/blas64/dgemv.v
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module blas
module blas64

import vsl.float.float64
import math
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module blas
module blas64

fn test_dgemv_no_trans_1() {
expected := [0.0, 0, 0, 0, 0]
Expand Down
3 changes: 2 additions & 1 deletion vlas/internal/blas/error.v → blas/blas64/error.v
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
module blas
module blas64

// Panic strings used during parameter checks.
// This list is duplicated in netlib/blas/netlib. Keep in sync.

pub const zero_incx = 'blas: zero x index increment'
pub const zero_incy = 'blas: zero y index increment'

Expand Down
2 changes: 1 addition & 1 deletion vlas/internal/blas/level1f64.v → blas/blas64/level1f64.v
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module blas
module blas64

import vsl.float.float64
import math
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module blas
module blas64

import vsl.float.float64

Expand Down
2 changes: 1 addition & 1 deletion vlas/internal/blas/level2f64.v → blas/blas64/level2f64.v
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module blas
module blas64

import math
import vsl.float.float64
Expand Down
2 changes: 1 addition & 1 deletion vlas/internal/blas/level3f64.v → blas/blas64/level3f64.v
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module blas
module blas64

import vsl.float.float64
import math
Expand Down
2 changes: 1 addition & 1 deletion vlas/internal/blas/util.v → blas/blas64/util.v
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module blas
module blas64

// [SD]gemm behavior constants. These are kept here to keep them out of the
// way during single precision code genration.
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module vlas
module blas

#flag linux -O2 -I/usr/local/include -I/usr/lib
#flag linux -L/usr/local/lib -L/usr/lib
Expand All @@ -7,11 +7,9 @@ module vlas
// Intel, M1 brew, and MacPorts
#flag darwin -I/usr/local/opt/openblas/include -I/opt/homebrew/opt/openblas/include -I/opt/local/opt/openblas/include
#flag darwin -L/usr/local/opt/openblas/lib -L/opt/homebrew/opt/openblas/lib -L/opt/local/opt/openblas/lib
#flag darwin -L/usr/local/opt/lapack/lib -L/opt/homebrew/opt/lapack/lib -L/opt/local/opt/lapack/lib
#flag -I@VMODROOT
#flag -lopenblas -llapacke
#flag -lopenblas

$if macos {
#include <lapacke.h>
#include <cblas.h>
}
27 changes: 13 additions & 14 deletions vlas/conversions.v → blas/conversions.v
Original file line number Diff line number Diff line change
@@ -1,26 +1,25 @@
module vlas
module blas

import strconv
import math
import math.complex
import vsl.errors
import vsl.vlas.internal.blas
import vsl.blas.blas64

pub fn c_trans(trans bool) blas.Transpose {
return if trans { .trans } else { .no_trans }
}
// MemoryLayout is used to specify the memory layout of a matrix.
pub type MemoryLayout = blas64.MemoryLayout

pub fn c_uplo(up bool) blas.Uplo {
return if up { .upper } else { .lower }
}
// Transpose is used to specify the transposition of a matrix.
pub type Transpose = blas64.Transpose

fn l_uplo(up bool) u8 {
return if up { `U` } else { `L` }
}
// Uplo is used to specify whether the upper or lower triangle of a matrix is
pub type Uplo = blas64.Uplo

fn job_vlr(do_calc bool) rune {
return if do_calc { `V` } else { `N` }
}
// Diagonal is used to specify whether the diagonal of a matrix is unit or non-unit.
pub type Diagonal = blas64.Diagonal

// Side is used to specify whether a matrix is on the left or right side in a matrix-matrix multiplication.
pub type Side = blas64.Side

Comment on lines +21 to +22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding examples for Side.

To enhance understanding, consider adding examples or more detailed explanations on how the Side type is used in operations.

Would you like me to add examples or detailed usage documentation for this type?


Clarify documentation for Side type.

The comment for Side should be more detailed to explain its use in matrix-matrix multiplication contexts.

- // Side is used to specify whether a matrix is on the left or right side in a matrix-matrix multiplication.
+ // Side specifies the side of the operand matrix in a matrix-matrix multiplication, determining if the matrix is multiplied from the left or the right.
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Side is used to specify whether a matrix is on the left or right side in a matrix-matrix multiplication.
pub type Side = blas64.Side
// Side specifies the side of the operand matrix in a matrix-matrix multiplication, determining if the matrix is multiplied from the left or the right.
pub type Side = blas64.Side

Tip

Codebase Verification

Ensure All Utility Functions Have Unit Tests

The review comment suggests verifying that all utility functions in blas/conversions.v are covered by unit tests. The executed script did not find any test functions in blas/conversions.v.

  • Please add unit tests for the utility functions in blas/conversions.v to ensure their correctness, especially for edge cases like zero-sized matrices.
Analysis chain

Review of utility functions for matrix operations.

The utility functions for converting and printing matrices are well-implemented. However, ensure that all functions are covered by unit tests to verify their correctness, especially with edge cases like zero-sized matrices.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify unit tests cover all utility functions in `blas/conversions.v`.

# Test: Search for unit tests related to utility functions.
rg --type v $'fn test_'

Length of output: 50



Script:

#!/bin/bash
# Description: Verify unit tests cover all utility functions in `blas/conversions.v`.

# Step 1: Find all `.v` files in the repository.
fd -e v -t f > v_files.txt

# Step 2: Search for test functions in the found `.v` files.
while read -r file; do
  rg 'fn test_' "$file"
done < v_files.txt

Length of output: 20332

// slice_to_col_major converts nested slice into an array representing a col-major matrix
//
Expand Down
Loading
Loading