From 809ded74c99ac6883bc5b5e4c7a2da887131360c Mon Sep 17 00:00:00 2001 From: Denis Davydov Date: Wed, 11 May 2016 15:02:14 +0200 Subject: [PATCH 1/3] add functions for simple unit tests; refactor openblas to use them --- lib/spack/spack/package_test.py | 64 +++++++++++++++++++ .../builtin/packages/openblas/package.py | 37 ++--------- 2 files changed, 71 insertions(+), 30 deletions(-) create mode 100644 lib/spack/spack/package_test.py diff --git a/lib/spack/spack/package_test.py b/lib/spack/spack/package_test.py new file mode 100644 index 0000000000..4b021684ee --- /dev/null +++ b/lib/spack/spack/package_test.py @@ -0,0 +1,64 @@ +############################################################################## +# Copyright (c) 2016, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Written by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/llnl/spack +# Please also see the LICENSE file for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License (as published by +# the Free Software Foundation) version 2.1 dated February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +from spack import * +import os + +def compile_c_and_execute(source_file,include_flags, link_flags): + """Compile C @p source_file with @p include_flags and @p link_flags, + run and return the output. + """ + cc = which('cc') + flags = include_flags + flags.extend([source_file]) + cc('-c', *flags) + name = os.path.splitext(os.path.basename(source_file))[0] + cc('-o', "check", "%s.o" % name, + *link_flags) + + check = Executable('./check') + return check(return_output=True) + + +def compare_output(current_output, blessed_output): + """Compare blessed and current output of executables.""" + if not (current_output == blessed_output): + print "Produced output does not match expected output." + print "Expected output:" + print '-'*80 + print blessed_output + print '-'*80 + print "Produced output:" + print '-'*80 + print current_output + print '-'*80 + raise RuntimeError("Ouput check failed. See spack_output.log for details") + + +def compare_output_file(current_output, blessed_output_file): + """Same as above, but when the blessed output is given as a file.""" + with open(blessed_output_file, 'r') as f: + blessed_output = f.read() + + compare_output(current_output,blessed_output) diff --git a/var/spack/repos/builtin/packages/openblas/package.py b/var/spack/repos/builtin/packages/openblas/package.py index d147533491..47b30181a8 100644 --- a/var/spack/repos/builtin/packages/openblas/package.py +++ b/var/spack/repos/builtin/packages/openblas/package.py @@ -1,4 +1,5 @@ from spack import * +from spack.package_test import * import sys import os import shutil @@ -94,42 +95,18 @@ def setup_dependent_package(self, module, dspec): self.spec.lapack_shared_lib = self.spec.blas_shared_lib def check_install(self, spec): - # TODO: Pull this out to the framework function which recieves a pair of xyz.c and xyz.output - print "Checking Openblas installation..." source_file = join_path(os.path.dirname(self.module.__file__), 'test_cblas_dgemm.c') - output_file = join_path(os.path.dirname(self.module.__file__), - 'test_cblas_dgemm.output') + blessed_file = join_path(os.path.dirname(self.module.__file__), + 'test_cblas_dgemm.output') - with open(output_file, 'r') as f: - expected = f.read() - - cc = which('cc') - cc('-c', "-I%s" % join_path(spec.prefix, "include"), source_file) + include_flags = ["-I%s" % join_path(spec.prefix, "include")] link_flags = ["-L%s" % join_path(spec.prefix, "lib"), "-llapack", "-lblas", - "-lpthread" - ] + "-lpthread"] if '+openmp' in spec: link_flags.extend([self.compiler.openmp_flag]) - cc('-o', "check", "test_cblas_dgemm.o", - *link_flags) - try: - check = Executable('./check') - output = check(return_output=True) - except: - output = "" - success = output == expected - if not success: - print "Produced output does not match expected output." - print "Expected output:" - print '-'*80 - print expected - print '-'*80 - print "Produced output:" - print '-'*80 - print output - print '-'*80 - raise RuntimeError("Openblas install check failed") + output = compile_c_and_execute(source_file,include_flags,link_flags) + compare_output_file(output,blessed_file) From 361b6290341e1248b3c5a5c3895b03610cb7ae3f Mon Sep 17 00:00:00 2001 From: Denis Davydov Date: Wed, 11 May 2016 15:09:48 +0200 Subject: [PATCH 2/3] openblas: make unit test more robust by outputing only the solution vector from dgesv --- .../repos/builtin/packages/openblas/test_cblas_dgemm.c | 2 +- .../repos/builtin/packages/openblas/test_cblas_dgemm.output | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/var/spack/repos/builtin/packages/openblas/test_cblas_dgemm.c b/var/spack/repos/builtin/packages/openblas/test_cblas_dgemm.c index 3813a23b69..2cb90fb883 100644 --- a/var/spack/repos/builtin/packages/openblas/test_cblas_dgemm.c +++ b/var/spack/repos/builtin/packages/openblas/test_cblas_dgemm.c @@ -43,7 +43,7 @@ int main(void) { int ldb = 3; dgesv_(&n,&nrhs, &m[0], &lda, ipiv, &x[0], &ldb, &info); for (i=0; i<3; ++i) - printf("%5.1f %3d\n", x[i], ipiv[i]); + printf("%5.1f\n", x[i]); return 0; } diff --git a/var/spack/repos/builtin/packages/openblas/test_cblas_dgemm.output b/var/spack/repos/builtin/packages/openblas/test_cblas_dgemm.output index 9c235e314f..01404462c4 100644 --- a/var/spack/repos/builtin/packages/openblas/test_cblas_dgemm.output +++ b/var/spack/repos/builtin/packages/openblas/test_cblas_dgemm.output @@ -7,6 +7,6 @@ 5.000000 -1.000000 3.000000 - -0.3 1 - 3.0 1499101120 - -3.0 32767 + -0.3 + 3.0 + -3.0 From eba264fcd0701b2172dc39d2616cb2f90b4d8578 Mon Sep 17 00:00:00 2001 From: Denis Davydov Date: Wed, 11 May 2016 15:45:57 +0200 Subject: [PATCH 3/3] fix formatting --- lib/spack/spack/package_test.py | 16 +++++---- .../builtin/packages/openblas/package.py | 34 +++++++++++-------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/lib/spack/spack/package_test.py b/lib/spack/spack/package_test.py index 4b021684ee..9c15e3f5d0 100644 --- a/lib/spack/spack/package_test.py +++ b/lib/spack/spack/package_test.py @@ -25,7 +25,8 @@ from spack import * import os -def compile_c_and_execute(source_file,include_flags, link_flags): + +def compile_c_and_execute(source_file, include_flags, link_flags): """Compile C @p source_file with @p include_flags and @p link_flags, run and return the output. """ @@ -46,14 +47,15 @@ def compare_output(current_output, blessed_output): if not (current_output == blessed_output): print "Produced output does not match expected output." print "Expected output:" - print '-'*80 + print '-' * 80 print blessed_output - print '-'*80 + print '-' * 80 print "Produced output:" - print '-'*80 + print '-' * 80 print current_output - print '-'*80 - raise RuntimeError("Ouput check failed. See spack_output.log for details") + print '-' * 80 + raise RuntimeError("Ouput check failed.", + "See spack_output.log for details") def compare_output_file(current_output, blessed_output_file): @@ -61,4 +63,4 @@ def compare_output_file(current_output, blessed_output_file): with open(blessed_output_file, 'r') as f: blessed_output = f.read() - compare_output(current_output,blessed_output) + compare_output(current_output, blessed_output) diff --git a/var/spack/repos/builtin/packages/openblas/package.py b/var/spack/repos/builtin/packages/openblas/package.py index 47b30181a8..cd8e3755ce 100644 --- a/var/spack/repos/builtin/packages/openblas/package.py +++ b/var/spack/repos/builtin/packages/openblas/package.py @@ -1,8 +1,7 @@ from spack import * from spack.package_test import * -import sys import os -import shutil + class Openblas(Package): """OpenBLAS: An optimized BLAS library""" @@ -14,9 +13,9 @@ class Openblas(Package): version('0.2.16', 'fef46ab92463bdbb1479dcec594ef6dc') version('0.2.15', 'b1190f3d3471685f17cfd1ec1d252ac9') - variant('shared', default=True, description="Build shared libraries as well as static libs.") + variant('shared', default=True, description="Build shared libraries as well as static libs.") # NOQA: ignore=E501 variant('openmp', default=False, description="Enable OpenMP support.") - variant('fpic', default=True, description="Build position independent code") + variant('fpic', default=True, description="Build position independent code") # NOQA: ignore=E501 # virtual dependency provides('blas') @@ -48,11 +47,11 @@ def install(self, spec, prefix): # Add support for OpenMP if '+openmp' in spec: - # Note: Apple's most recent Clang 7.3.0 still does not support OpenMP. - # What is worse, Openblas (as of 0.2.18) hardcoded that OpenMP cannot + # Openblas (as of 0.2.18) hardcoded that OpenMP cannot # be used with any (!) compiler named clang, bummer. if spec.satisfies('%clang'): - raise InstallError('OpenBLAS does not support OpenMP with clang!') + raise InstallError('OpenBLAS does not support ', + 'OpenMP with clang!') make_defs += ['USE_OPENMP=1'] @@ -69,29 +68,34 @@ def install(self, spec, prefix): symlink('libopenblas.a', 'blas.a') symlink('libopenblas.a', 'libblas.a') if '+shared' in spec: - symlink('libopenblas.%s' % dso_suffix, 'libblas.%s' % dso_suffix) + symlink('libopenblas.%s' % dso_suffix, + 'libblas.%s' % dso_suffix) # Lapack virtual package should provide liblapack.a with working_dir(prefix.lib): symlink('libopenblas.a', 'liblapack.a') if '+shared' in spec: - symlink('libopenblas.%s' % dso_suffix, 'liblapack.%s' % dso_suffix) + symlink('libopenblas.%s' % dso_suffix, + 'liblapack.%s' % dso_suffix) # Openblas may pass its own test but still fail to compile Lapack - # symbols. To make sure we get working Blas and Lapack, do a small test. + # symbols. To make sure we get working Blas and Lapack, do a small + # test. self.check_install(spec) - def setup_dependent_package(self, module, dspec): # This is WIP for a prototype interface for virtual packages. # We can update this as more builds start depending on BLAS/LAPACK. - libdir = find_library_path('libopenblas.a', self.prefix.lib64, self.prefix.lib) + libdir = find_library_path('libopenblas.a', + self.prefix.lib64, + self.prefix.lib) self.spec.blas_static_lib = join_path(libdir, 'libopenblas.a') self.spec.lapack_static_lib = self.spec.blas_static_lib if '+shared' in self.spec: - self.spec.blas_shared_lib = join_path(libdir, 'libopenblas.%s' % dso_suffix) + self.spec.blas_shared_lib = join_path(libdir, 'libopenblas.%s' % + dso_suffix) self.spec.lapack_shared_lib = self.spec.blas_shared_lib def check_install(self, spec): @@ -108,5 +112,5 @@ def check_install(self, spec): if '+openmp' in spec: link_flags.extend([self.compiler.openmp_flag]) - output = compile_c_and_execute(source_file,include_flags,link_flags) - compare_output_file(output,blessed_file) + output = compile_c_and_execute(source_file, include_flags, link_flags) + compare_output_file(output, blessed_file)