Skip to content
This repository was archived by the owner on Nov 24, 2018. It is now read-only.

Conversation

@btracey
Copy link
Member

@btracey btracey commented May 6, 2015

No description provided.

@kortschak
Copy link
Member

You'll need to edit .travis.yaml to point to the correct location at line 42 s/clapack/cgo/.

Copy link
Member

Choose a reason for hiding this comment

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

Prefix with "Package".

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@btracey
Copy link
Member Author

btracey commented May 7, 2015

PTAL

@kortschak
Copy link
Member

kortschak commented May 7, 2015 via email

@btracey
Copy link
Member Author

btracey commented May 7, 2015

Travis was updated to swap clapack for cgo. It's still failing (obviously). Before it was giving a "pushd clapack" failure (or something like that). Now it's failing with

The command "go test -x -v ./..." exited with 2.
0.86s
$ diff <(gofmt -d .) <("")
/home/travis/build.sh: line 41: : command not found

There are a bunch of lapack.Job warnings as well (not sure what they mean). Is the problem that there are no go files in the cgo folder (just headers and a perl script?). Those have been there for a while, but I've never tried to integrate a c lapack installation.

@kortschak
Copy link
Member

kortschak commented May 7, 2015 via email

lapack.go Outdated
Copy link
Member

Choose a reason for hiding this comment

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

This was needed by cgo/lapack.

@kortschak
Copy link
Member

OK, figured out why it wasn't building: There are types defined in lapack/lapack.go that are required by lapack/cgo/lapack.go

@btracey
Copy link
Member Author

btracey commented May 8, 2015

Okay, cgo now compiles on my machine. I hacked the perl script in a way that's not pretty, but we'll need to update the script anyway at some point to use go generate, and have the right return types, etc.

@kortschak
Copy link
Member

Are all the in returns in LAPACK 0=success !0=failure?

@kortschak
Copy link
Member

Changes for genLapack.pl

diff --git a/cgo/genLapack.pl b/cgo/genLapack.pl
index 8c15042..07de5ef 100755
--- a/cgo/genLapack.pl
+++ b/cgo/genLapack.pl
@@ -57,12 +57,14 @@ const (
        rowMajor order = 101 + iota
        colMajor
 )
- 
+
 func init() {
          _ = lapack.Complex128(Lapack{})
          _ = lapack.Float64(Lapack{})
 }

+func isZero(ret C.int) bool { return ret == 0 }
+
 EOH

 $/ = undef;
@@ -110,11 +112,18 @@ our %typeConv = (
        "LAPACK_C_SELECT2" => "Select2Complex64",
        "LAPACK_Z_SELECT1" => "Select1Complex128",
        "LAPACK_Z_SELECT2" => "Select2Complex128",
-       "void" => ""
+       "void" => "",
+
+       "lapack_int_return_type" => "bool",
+       "lapack_int_return" => "isZero",
+       "float_return_type" => "float32",
+       "float_return" => "float32",
+       "double_return_type" => "float64",
+       "double_return" => "float64"
 );

 foreach my $line (@lines) {
-       process($line); 
+       process($line);
 }

 close($golapack);
@@ -162,14 +171,15 @@ sub processProto {
                $gofunc = ucfirst $func;
        }

-       my $GoRet = $typeConv{$ret};
+       my $GoRet = $typeConv{$ret."_return"};
+       my $GoRetType = $typeConv{$ret."_return_type"};
        my $complexType = $func;
        $complexType =~ s/.*_[isd]?([zc]).*/$1/;
        my ($params,$bp) = processParamToGo($func, $paramList, $complexType);
        if ($params eq "") {
                return
        }
-       print $golapack "func (Lapack) ".$gofunc."(".$params.") ".$GoRet."{\n";
+       print $golapack "func (Lapack) ".$gofunc."(".$params.") ".$GoRetType."{\n";
        print $golapack "\t";
        if ($ret ne 'void') {
                print $golapack "\n".$bp."\n"."return ".$GoRet."(";

@btracey
Copy link
Member Author

btracey commented May 8, 2015

Sorry, how do I apply that patch? I reverted back to the original genLapack.pl. I copied the text in the block above, saved it in cgo as patch.diff, and ran git apply -v patch.diff. I get the following output:

brendan:~/Documents/mygo/src/github.com/gonum/lapack/cgo$ git apply -v patch.diff 
Checking patch cgo/genLapack.pl...
error: while searching for:
       rowMajor order = 101 + iota
       colMajor
)

func init() {
         _ = lapack.Complex128(Lapack{})
         _ = lapack.Float64(Lapack{})
}

EOH

$/ = undef;

error: patch failed: cgo/genLapack.pl:57
error: cgo/genLapack.pl: patch does not apply

I don't understand the "error searching for" because those lines are in the file...

@btracey
Copy link
Member Author

btracey commented May 8, 2015

I'm not familiar with all of Lapack (learning as I go), but so far all of the returns are zero for success.

@btracey
Copy link
Member Author

btracey commented May 8, 2015

Okay, when I say "restored", I actually mean I copied and pasted in the old file. I wonder if that's part of the problem (where git is using a file index rather than just the characters?)

@kortschak
Copy link
Member

Yeah, that will lead to a conflict.

@kortschak
Copy link
Member

This last failure is due to removal of the test-coverage.sh file.

@btracey
Copy link
Member Author

btracey commented May 8, 2015

I thought we had removed all of those files. Do I remember incorrectly?

On May 8, 2015, at 4:47 AM, Dan Kortschak notifications@github.com wrote:

This last failure is due to removal of the test-coverage.sh file.


Reply to this email directly or view it on GitHub #11 (comment).

@kortschak
Copy link
Member

kortschak commented May 8, 2015 via email

@btracey
Copy link
Member Author

btracey commented May 8, 2015

There we go. Sorry for making the process difficult.

@kortschak
Copy link
Member

kortschak commented May 8, 2015 via email

@btracey
Copy link
Member Author

btracey commented May 8, 2015

PTAL

@kortschak
Copy link
Member

LGTM

Responded to PR comments

modified travis file

Changed input and output types

added back needed types by cgo

Fixed perl script so it compiles

Changes to genLapack to allow compilation

Reinstate test-coverage.sh
btracey added a commit that referenced this pull request May 8, 2015
Rearrange Lapack to be like BLAS. Implement cholesky decomposition
@btracey btracey merged commit b231f5c into master May 8, 2015
@btracey btracey deleted the adddpotrf branch May 8, 2015 23:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants