bugGNU Octave - Bugs: bug #32966, spdiags is slow and buggy, diag is...

 
 
Show feedback again

bug #32966: spdiags is slow and buggy, diag is not compatible

Submitted by:  Marco Caliari <caliari>
Submitted on:  Thu 31 Mar 2011 01:21:38 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: Patch SubmittedAssigned to: None
Originator Name: Open/Closed: Open
Release: 3.4.0Operating System: GNU/Linux

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Thu 14 Mar 2013 07:08:21 AM UTC, comment #4:

I'm sorry Rik, but I won't be able to contribute to Octave for the next few months. I'm looking forward to start again.

Thanks,

Marco

Marco Caliari <caliari>
Project Member
Tue 12 Mar 2013 11:19:16 PM UTC, comment #3:

Marco, this bug report is 1.5 years old now. Do you want to rebase your patches onto a recent Mercurial changeset and close this report?

Rik <rik5>
Project Member
Fri 30 Sep 2011 04:06:19 PM UTC, comment #2:

Marco,

This wasn't the case when you filed this bug report, but you are part of the Octave team now and have commit privileges to Mercurial. Do you want to just apply the patches in this bug report yourself and close it? The performance gains seem impressive.

Rik <rik5>
Project Member
Fri 08 Apr 2011 01:54:26 PM UTC, comment #1:

I'm submitting two patches: diag.patch fixes the compatibility issue 3). spdiags.m.patch rewrites the function calls spdiags (A) and spdiags(A, C) and uses diag to extract diagonals. This fixes issues 1) and 2). Infact, using the testfile testspdiags.m (which creates sparse matrices of random sizes and extract random diagonals), I see

+verbatim-

> testspdiags(10) % random sparse matrices up to size 10

warning: spdiagsorig failed
...
warning: spdiagsorig failed
cpuorigtot = 10.013
cputot = 0.85205
-verbatim-

Because of issue 2) in the original spdiags, sometimes you get the warning. Two compatibility issues remain: number 4) (which I think is a bug in ml). Then, if A is a vector, ml gives an error, whereas the original spdiags in Octave (and mine) works. When you call spdiags with 1 or 2 input args you want to extract diagonals (no ambiguity as in diag): so a vector is just a rectangular matrix.

Marco

(file #23138, file #23139, file #23140)

Marco Caliari <caliari>
Project Member
Thu 31 Mar 2011 01:21:38 PM UTC, original submission:

Dear maintainers,

stimulated by the comment
in spdiags.m, I found the following:

1) diag is much faster then spdiags in extracting diagonals. I get

2) spdiags fails in the following case:
whereas it works with [1,1;1,1].

3) diag is not compatible:
whereas Matlab gives empty matrix 0-by-1.

If 3) is fixed, I can easily rewrite spdiags.m to avoid problems 2) and 1) (without testing if the diagonal is out of range). However, I'm not clever and I cannot do it without a loop :-(

4) there is another incompatible behaviour:
whereas in Matlab
I do not understand this Matlab (7.6.0) behaviour.

If we take a decision about 4) and someone fixes 3), I can work on 1) and 2).

Cheers,

Marco

Marco Caliari <caliari>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #23138:  diag.patch added by caliari (1kB - text/x-patch)
file #23139:  spdiags.m.patch added by caliari (2kB - text/x-patch)
file #23140:  testspdiags.m added by caliari (612B - text/x-objcsrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by rik5 (Updated the item)
  • -unavailable- added by caliari (Submitted the item)
  •  

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 04 Aug 2011 05:51:13 AM UTCrik5StatusNone=>Patch Submitted
    Fri 08 Apr 2011 01:54:26 PM UTCcaliariAttached File-=>Added diag.patch, #23138
      Attached File-=>Added spdiags.m.patch, #23139
      Attached File-=>Added testspdiags.m, #23140
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup