I found a typo in Basic/MatrixOps/MatrixOps.pm last night & patched it:

-  if(exists ($opt->{u}) and (ref $opt->{lu} eq 'ARRAY')) {
+  if(exists ($opt->{lu}) and (ref $opt->{lu} eq 'ARRAY')) {

Would somebody shepherd me through submitting the patch correctly?
My first-cut attempt is below.  Any guidance is greatly appreciated :-)

The typo causes det($opt) to ignore a cached lu_decomp in the $opt hashref.
My patch started out as 2 patches, a test and a fix, then became 4 because
MatrixOps.pm was in .gitignore and not tracked.  The patches are:

    0001-test-matrixops-det-use-of-cached-lu_decomp.patch
    0002-don-t-gitignore-MatrixOps.pm-with-det-lu-bug.patch
    0003-initial-commit-of-MatrixOps.pm-with-det-lu-bug.patch
    0004-matrixops-det-uses-cached-lu_decomp.patch

The 0003-initial-commit patch is not listed below because it is big and
just adds the existing MatrixOps.pm to the git repo.

I emailed the patches to myself and applied them with "git am" to a fresh
"git clone https://github.com/PDLPorters/pdl.git"; after "perl Makefile.PL",
"make", and "make test".  I ran "make test" again after the patches were
applied, and manually verified the cached lu_decomp was working.

To get the 0003-initial-commit patch to apply, I had to first
"rm Basic/MatrixOps/MatrixOps.pm" from the working directory.

Below are patches 0001, 0002, & 0004.  If I should email them (and 0003)
to somebody, let me know.  Thanks, --Jerry


[PATCH 1/4] test matrixops det use of cached lu_decomp
---
 t/matrixops.t | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/t/matrixops.t b/t/matrixops.t
index 36042a0..719d25b 100644
--- a/t/matrixops.t
+++ b/t/matrixops.t
@@ -1,5 +1,5 @@
 use PDL::LiteF;
-use Test::More tests => 38;
+use Test::More tests => 42;
 use Test::Exception;
 use Config;

@@ -120,6 +120,17 @@ ok(!defined $b2, "inv of singular matrix undefined if 
s=>1");
 }

 {
+### Check that det will save lu_decomp and reuse it
+my $m1 = pdl [[1,2],[3,4]];  # det -2
+my $opt1 = {lu=>undef};
+ok($m1->det($opt1) == -2, "det([[1,2],[3,4]]");
+ok($opt1->{lu}[0]->index2d(0,0) == 3, "set lu");
+my $m2 = pdl [[2,1],[4,3]];  # det 2
+ok($m2->det == 2, "det([[2,1],[3,4]]");
+ok($m2->det($opt1) == -2, "correctly used wrong lu");
+}
+
+{
 ### Check threaded determinant -- simultaneous recursive det of four 4x4's
 my $pa = pdl([3,4,5,6],[6,7,8,9],[9,0,7,6],[4,3,2,0]); # det=48
 my $pb = pdl([1,0,0,0],[0,1,0,0],[0,0,1,0],[0,0,0,1]); # det=1
-- 
2.7.4


[PATCH 2/4] don't gitignore MatrixOps.pm with det lu bug
---
 .gitignore | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 2f0bb60..d98bc4c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -102,7 +102,7 @@ Basic/Math/Math.c
 Basic/Math/Math.pm
 Basic/Math/Math.xs
 Basic/MatrixOps/MatrixOps.c
-Basic/MatrixOps/MatrixOps.pm
+# Basic/MatrixOps/MatrixOps.pm
 Basic/MatrixOps/MatrixOps.xs
 Basic/Ops/Ops.c
 Basic/Ops/Ops.pm
-- 
2.7.4


[PATCH 4/4] matrixops det uses cached lu_decomp
---
 Basic/MatrixOps/MatrixOps.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Basic/MatrixOps/MatrixOps.pm b/Basic/MatrixOps/MatrixOps.pm
index b7e4a29..6d686fa 100644
--- a/Basic/MatrixOps/MatrixOps.pm
+++ b/Basic/MatrixOps/MatrixOps.pm
@@ -343,7 +343,7 @@ sub det {
   $opt = {} unless defined($opt);

   my($lu,$perm,$par);
-  if(exists ($opt->{u}) and (ref $opt->{lu} eq 'ARRAY')) {
+  if(exists ($opt->{lu}) and (ref $opt->{lu} eq 'ARRAY')) {
     ($lu,$perm,$par) =  @{$opt->{lu}};
   } else {
     ($lu,$perm,$par) = lu_decomp($a);
-- 
2.7.4

Attachment: 0x1AB43E74.asc
Description: application/pgp-keys

Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
pdl-devel mailing list
pdl-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/pdl-devel

Reply via email to