bugGNU Octave - Bugs: bug #59226, Function colperm is broken for...

 
 

bug #59226: Function colperm is broken for matrices with empty columns

Submitter:  None
Submitted:  Tue 06 Oct 2020 08:53:46 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Marcel Jacobse Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 6.0.90
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 08 Oct 2020 09:51:26 AM UTC, comment #6: 

I added Marcel to the list of contributors here:
https://hg.savannah.gnu.org/hgweb/octave/rev/6f842b870f3c

Markus Mützel <mmuetzel>
Group administrator
Wed 07 Oct 2020 09:59:23 PM UTC, comment #5: 

I verified the fix so I'm going to close this report.  Markus can handle the last bit of adding Marcel to the contributors list in the manual.

Rik <rik5>
Group administrator
Wed 07 Oct 2020 08:55:59 PM UTC, comment #4: 

This is an actual bug fix so I grafted the change from the stable branch back to the development branch.  I also made a small change to the documentation to follow Octave guidelines (https://hg.savannah.gnu.org/hgweb/octave/rev/54b9fcbefd73).

Rik <rik5>
Group administrator
Tue 06 Oct 2020 08:35:55 PM UTC, comment #3: 

comment #2:

> Would you like to be added to the list of contributors that appears at the front of the manual?


Sure, I wouldn't mind that. Thanks for the quick answer.

Marcel

Anonymous
Tue 06 Oct 2020 08:22:09 PM UTC, comment #2: 

This is a bug that goes back to when this function was introduced in 2005.

I added a BIST with the motivating example from comment #0 and pushed your patch here:
https://hg.savannah.gnu.org/hgweb/octave/rev/20f31c0b0132

Would you like to be added to the list of contributors that appears at the front of the manual?

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Tue 06 Oct 2020 05:21:26 PM UTC, comment #1: 

Confirmed. Verified that the patch from the OP fixes the problem.

Anonymous
Tue 06 Oct 2020 08:53:46 AM UTC, original submission:  

Example:

>> colperm([1,0;0,0])
ans = 1


Expected result:

ans =

   2   1


Empty columns are basically completely ignored, so that even the dimension of the returned permutation vector is not correct. This is the code for the function in version 5.2.0:

function p = colperm (s)

  if (nargin != 1)
    print_usage ();
  endif

  [i, j] = find (s);
  idx = find (diff ([j; Inf]) != 0);
  [~, p] = sort (idx - [0; idx(1:(end-1))]);

endfunction


The problem is that empty columns will not appear in that vector of column indices 'j', and therefore will not appear in the vector of column-end indices 'idx'. I can not think of a simple solution for this problem with this solution via 'find'.

Instead, I propose to implement this function via 'sum' instead. See the attached patch. I tested with a few matrices from the SuiteSparse Matrix Collection on my Windows machine with an i7-9750H and the new implementation also tends to be as fast or even faster than the current implementation in most cases. Additionally, the new implementation returns a column vector just like the other permutation functions ('amd' and so on). That is another minor bug with the current implementation of 'colperm'.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #49924:  colperm_fix.patch added by None (898B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-10-07 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2020-10-07 rik5 Item GroupNone Incorrect Result
        Release5.2.0 6.0.90
    2020-10-06 mmuetzel StatusNone Ready For Test
    2020-10-06 None Attached File- Added colperm_fix.patch, #49924

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code