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

Fix tests on macOS #184

Merged
merged 4 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 2 additions & 21 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,6 @@ jobs:

- name: Execute Tests using Pure V Backend with Pure C Blas
run: ~/.vmodules/vsl/bin/test ${{ matrix.flags }} --use-cblas
if: ${{ matrix.os != 'ubuntu-18.04' }}

- name: Execute Tests using Pure V Backend and Garbage Collection enabled
run: ~/.vmodules/vsl/bin/test ${{ matrix.flags }} --use-gc boehm

- name: Execute Tests using Pure V Backend with Pure C Blas and Garbage Collection enabled
run: ~/.vmodules/vsl/bin/test ${{ matrix.flags }} --use-cblas --use-gc boehm
if: ${{ matrix.os != 'ubuntu-18.04' }}
Comment on lines -78 to -79
Copy link
Member Author

@ttytm ttytm Oct 22, 2023

Choose a reason for hiding this comment

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

Removes apparently obsolete

  • gc boehm is the default mode, but I might be missing something that it was called explicitly here. Else, there is no difference with the run above.
  • ubuntu-18.04 is not in the matrix, support for such runners was deprecated


run-tests-on-macos:
runs-on: ${{ matrix.os }}
Expand Down Expand Up @@ -118,18 +110,7 @@ jobs:
run: mv ./vsl ~/.vmodules

- name: Execute Tests using Pure V Backend
# TODO: Remove continue-on-error when CI is fixed for macOS
continue-on-error: true
run: ~/.vmodules/vsl/bin/test
run: ~/.vmodules/vsl/bin/test --skip-examples

- name: Execute Tests using Pure V Backend with Pure C Blas
continue-on-error: true
run: ~/.vmodules/vsl/bin/test --use-cblas

- name: Execute Tests using Pure V Backend and Garbage Collection enabled
continue-on-error: true
run: ~/.vmodules/vsl/bin/test --use-gc boehm

- name: Execute Tests using Pure V Backend with Pure C Blas and Garbage Collection enabled
continue-on-error: true
run: ~/.vmodules/vsl/bin/test --use-cblas --use-gc boehm
run: ~/.vmodules/vsl/bin/test --use-cblas --skip-examples
Copy link
Member

Choose a reason for hiding this comment

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

why are we using the flag --skip-examples? 👀

Copy link
Member Author

Choose a reason for hiding this comment

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

Tests work, but not all examples build on mac currently.

Thought we can re-enable it when they do.

4 changes: 2 additions & 2 deletions float/float64/ge_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ struct GeTest {
}

fn test_ger() {
tol := 1e-15
tol := $if macos { 7e-15 } $else { 1e-15 }

x_gd_val, y_gd_val, a_gd_val := -0.5, 1.5, 10
gd_ln := 4
Expand All @@ -87,7 +87,7 @@ fn test_ger() {
alpha := 1.0
ger(u32(m), u32(n), alpha, x, 1, y, 1, mut a, u32(n))
for i, w in test.want {
assert tolerance(a[i], w, tol)
assert tolerance(a[i], w, tol), 'got: ${a[i]}, want: ${w}, tol: ${tol}'
}
}
}
Expand Down
17 changes: 13 additions & 4 deletions iter/ranges_test.v
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module iter

import math

fn test_int_iter() {
args := [[i64(27), 11, -5], [i64(13), 1, -6], [i64(-19), -35, -6],
[i64(-88), -29, 8], [i64(17), -94, 2], [i64(30), 45, 6],
Expand Down Expand Up @@ -52,10 +54,12 @@ fn test_float_iter() {
for i, values in args {
r := FloatIter.new(start: values[0], stop: values[1], step: values[2])!
for j, n in r {
if !n.eq_epsilon(expected[i][j]) {
println('${n}, ${expected[i][j]}')
$if macos {
tol := 2e-15
assert math.tolerance(expected[i][j], n, tol), 'got: ${n}, want: ${expected[i][j]}, tol: ${tol}'
} $else {
assert n.eq_epsilon(expected[i][j]), 'got: ${n}, want: ${expected[i][j]}'
}
assert n.eq_epsilon(expected[i][j])
}
assert r.len == expected[i].len
}
Expand Down Expand Up @@ -90,7 +94,12 @@ fn test_linear_iter() {
for i, lim in limits {
l := LinearIter.new(start: lim[0], stop: lim[1], len: lens[i], endpoint: endpoints[i])!
for j, n in l {
assert n.eq_epsilon(expected[i][j])
$if macos {
tol := 2e-15
assert math.tolerance(expected[i][j], n, tol), 'got: ${n}, want: ${expected[i][j]}'
} $else {
assert n.eq_epsilon(expected[i][j]), 'got: ${n}, want: ${expected[i][j]}'
}
}
assert l.len == expected[i].len
}
Expand Down
6 changes: 5 additions & 1 deletion la/matrix_ops_test.v
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ fn test_det() {
[4.0, -2.0, 5.0],
[2.0, 8.0, 7.0],
])
assert matrix_det(mat_b) == -306
$if macos {
assert math.tolerance(matrix_det(mat_b), -306, la.matrix_tests_tol)
} $else {
assert matrix_det(mat_b) == -306
}
}

fn test_matrix_inv_small() {
Expand Down
6 changes: 6 additions & 0 deletions vlas/cflags_d_cblas.v
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,11 @@ module vlas
#flag windows -lgfortran
#flag darwin -I/usr/local/opt/openblas/include
#flag darwin -L/usr/local/opt/openblas/lib
#flag darwin -L/usr/local/opt/lapack/lib
#flag -I@VMODROOT
#flag -lopenblas -llapacke

$if macos {
#include <lapacke.h>
#include <cblas.h>
}
4 changes: 4 additions & 0 deletions vlas/cflags_notd_cblas.v
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ module vlas
#flag darwin -L/usr/local/opt/lapack/lib
#flag -I@VMODROOT
#flag -llapacke

$if macos {
#include <lapacke.h>
}
18 changes: 4 additions & 14 deletions vlas/conversions.v
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,19 @@ import vsl.errors
import vsl.vlas.internal.vblas

pub fn c_trans(trans bool) vblas.Transpose {
if trans {
return .trans
}
return .no_trans
return if trans { .trans } else { .no_trans }
}

pub fn c_uplo(up bool) vblas.Uplo {
if up {
return .upper
}
return .lower
return if up { .upper } else { .lower }
}

fn l_uplo(up bool) u8 {
return if up { `U` } else { `L` }
}

fn job_vlr(do_calc bool) &char {
return &char(if do_calc {
'V'
} else {
'N'
}.str)
fn job_vlr(do_calc bool) rune {
return if do_calc { `V` } else { `N` }
}

// slice_to_col_major converts nested slice into an array representing a col-major matrix
Expand Down
12 changes: 3 additions & 9 deletions vlas/lapack.v → vlas/lapack_common.v
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ fn C.LAPACKE_dpotrf(matrix_layout vblas.MemoryLayout, up u32, n int, a &f64, lda

fn C.LAPACKE_dgeev(matrix_layout vblas.MemoryLayout, calc_vl &char, calc_vr &char, n int, a &f64, lda int, wr &f64, wi &f64, vl &f64, ldvl_ int, vr &f64, ldvr_ int) int

fn C.LAPACKE_dlange(matrix_layout vblas.MemoryLayout, norm &char, m int, n int, a &f64, lda int, work &f64) f64

fn C.LAPACKE_dsyev(matrix_layout vblas.MemoryLayout, jobz &char, uplo &char, n int, a &f64, lda int, w &f64, work &f64, lwork int) int
fn C.LAPACKE_dsyev(matrix_layout vblas.MemoryLayout, jobz byte, uplo byte, n int, a &f64, lda int, w &f64, work &f64, lwork int) int

fn C.LAPACKE_dgebal(matrix_layout vblas.MemoryLayout, job &char, n int, a &f64, lda int, ilo int, ihi int, scale &f64) int

Expand Down Expand Up @@ -191,14 +189,10 @@ pub fn dgeev(calc_vl bool, calc_vr bool, n int, mut a []f64, lda int, wr []f64,
ldvr = 1
}
unsafe {
info := C.LAPACKE_dgeev(.row_major, job_vlr(calc_vl), job_vlr(calc_vr), n, &a[0],
lda, &wr[0], &wi[0], &vvl, ldvl, &vvr, ldvr)
info := C.LAPACKE_dgeev(.row_major, &char(job_vlr(calc_vl).str().str), &char(job_vlr(calc_vr).str().str),
n, &a[0], lda, &wr[0], &wi[0], &vvl, ldvl, &vvr, ldvr)
if info != 0 {
errors.vsl_panic('lapack failed', .efailed)
}
}
}

pub fn dlange(norm &char, m int, n int, a []f64, lda int, work []f64) f64 {
return unsafe { C.LAPACKE_dlange(.row_major, norm, m, n, &a[0], lda, &work[0]) }
}
11 changes: 11 additions & 0 deletions vlas/lapack_default.c.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module vlas

import vsl.vlas.internal.vblas

fn C.LAPACKE_dlange(matrix_layout vblas.MemoryLayout, norm &char, m int, n int, a &f64, lda int, work &f64) f64

pub fn dlange(norm rune, m int, n int, a []f64, lda int, work []f64) f64 {
return unsafe {
C.LAPACKE_dlange(.row_major, &char(norm.str().str), m, n, &a[0], lda, &work[0])
}
}
Comment on lines +1 to +11
Copy link

Choose a reason for hiding this comment

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

The function dlange is a wrapper around the C function LAPACKE_dlange. It's good to see that you're using the unsafe block to call the C function, which is a good practice in V language when dealing with C interop. However, it's important to ensure that the parameters passed to the C function are valid and won't cause any undefined behavior. For instance, you're passing the address of the first element of the a and work arrays to the C function. If these arrays are empty, this will result in undefined behavior. You should add a check to ensure these arrays are not empty before calling the C function.

pub fn dlange(norm rune, m int, n int, a []f64, lda int, work []f64) f64 {
+	if a.len == 0 || work.len == 0 {
+		return 0.0
+	}
	return unsafe {
		C.LAPACKE_dlange(.row_major, &char(norm.str().str), m, n, &a[0], lda, &work[0])
	}
}

7 changes: 7 additions & 0 deletions vlas/lapack_macos.c.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module vlas

fn C.LAPACKE_dlange(norm &char, m int, n int, a &f64, lda int, work &f64) f64

pub fn dlange(norm rune, m int, n int, a []f64, lda int, work []f64) f64 {
return unsafe { C.LAPACKE_dlange(&char(norm.str().str), m, n, &a[0], lda, &work[0]) }
}
Loading