bugGNU Octave - Bugs: bug #54275, repelem error if a repeat...

 
 

bug #54275: repelem error if a repeat instruction is a vector that ends with 0

Submitter:  None
Submitted:  Mon 09 Jul 2018 09:38:53 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 4.4.0
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Mon 09 Jul 2018 06:34:31 PM UTC, comment #2: 

Confirmed.  As noted in the title, this is a problem only when the last position is a 0.

I would prefer not to call out to a second function (repelems) because there is a fair amount of interpreter overhead in calling a function.

Since methods 1 and 3 both work, the only question is performance.  I made a small benchmark


N = 1e4;

bm = zeros (N, 1);

base = [1:10];
rep  = [9:-1:0];

for i = 1:N
  tic;
  c = repelem (base, rep);
  bm(i) = toc;
endfor


The results are



Patch 1:
------------------------------------------------------------
octave:11> mean (bm)
ans =  0.00017877
octave:12> median (bm)
ans =  0.00016284
octave:13> std (bm)
ans =  0.000072822


Patch 3:
------------------------------------------------------------
octave:5> mean (bm)
ans =  0.00015290
octave:4> median (bm)
ans =  0.00013614
octave:6> std (bm)
ans =  0.000080379


Patch three is the winner (although we're only talking about 20 milliseconds).

I committed the patch to the stable branch here (http://hg.savannah.gnu.org/hgweb/octave/rev/9277b77dd28f).

Marking as fixed and closing report.

Rik <rik5>
Group administrator
Mon 09 Jul 2018 12:04:31 PM UTC, comment #1: 

The first solution in the original submission is a little ugly.
If we didn't want to use the second solution (using "repelems"),
here is a third solution.

--- a/scripts/general/repelem.m
+++ b/scripts/general/repelem.m
@@ -299,20 +299,19 @@

   else
     ## This works for a row or column vector.
-    ## idx2 output will be a row vector.

     ## Get ending position for each element item.
     idx_temp = cumsum (v);

-    ## Row vector with enough space for output
-    idx(1:idx_temp(end)) = 0;
-
     ## Set starting position of each element to 1.
-    idx(idx_temp(1:end-1) + 1) = 1;
+    idx(idx_temp + 1) = 1;

     ## Set starting position of each element to 1.
     idx(1) = 1;

+    ## Row vector with proper length for output
+    idx = idx(1:idx_temp(end));
+
     ## with prepared index
     idx = (find (v != 0))(cumsum (idx));


Besides, in fact, in the subfuntion prepareIdx, the variable "idx" is actually a row vector
except for the last line "idx = (find (v != 0))(cumsum (idx))".
The problem is that "find(v!=0)" might be a row or column vector according to the shape of "v".
If we really wanted the output "idx" to be a row vector,
we could change that line into "idx = find (v != 0)(:).'(cumsum (idx))".

Anonymous
Mon 09 Jul 2018 09:38:53 AM UTC, original submission:  

For example,

octave:1>  repelem(11:13, [1 3 0])
error: index (3): out of bound 2
error: called from
    repelem>prepareIdx at line 317 column 9
    repelem at line 204 column 12



One possible solution is as follow.
(Besides, note that the comment "idx2 output will be a row vector" is wrong; in fact the output can be either row or column vector.)

--- a/scripts/general/repelem.m
+++ b/scripts/general/repelem.m
@@ -299,7 +299,9 @@

   else
     ## This works for a row or column vector.
-    ## idx2 output will be a row vector.
+
+    ## Handle special case if v ends with 0.
+    v(end+1) = 1;

     ## Get ending position for each element item.
     idx_temp = cumsum (v);
@@ -316,6 +318,9 @@
     ## with prepared index
     idx = (find (v != 0))(cumsum (idx));

+    ## Handle special case if v ends with 0.
+    idx(end) = [];
+
   endif

 endfunction


Another solution is to use built-in function "repelems" directly.

--- a/scripts/general/repelem.m
+++ b/scripts/general/repelem.m
@@ -299,22 +299,10 @@

   else
     ## This works for a row or column vector.
-    ## idx2 output will be a row vector.
+    ## idx output will be a row vector.

-    ## Get ending position for each element item.
-    idx_temp = cumsum (v);
-
-    ## Row vector with enough space for output
-    idx(1:idx_temp(end)) = 0;
-
-    ## Set starting position of each element to 1.
-    idx(idx_temp(1:end-1) + 1) = 1;
-
-    ## Set starting position of each element to 1.
-    idx(1) = 1;
-
-    ## with prepared index
-    idx = (find (v != 0))(cumsum (idx));
+    idx_temp = 1:numel (v);
+    idx = repelems (idx_temp, [idx_temp; v(:).']);

   endif



And below is some proposed test case.

--- a/scripts/general/repelem.m
+++ d/scripts/general/repelem.m
@@ -341,6 +341,7 @@
 %!assert (repelem ([-1 0 1], [1 2 1]), [-1 0 0 1])
 %!assert (repelem ([-1 0 1]', [1 2 1]), [-1; 0; 0; 1])
 %!assert (repelem ([1 2 3 4 5]', [2 1 0 1 2]), [1 1 2 4 5 5]')
+%!assert (repelem ([1 2 3 4 5]', [2 1 2 1 0]), [1 1 2 3 3 4]')

 ## nargin == 3 tests
 %!assert (repelem ([1 0;0 -1], 2, 3),
@@ -384,6 +385,7 @@
 %!               1 1 1 1 1 1 0 0 0 1 1 1
 %!               1 1 1 1 1 1 0 0 0 1 1 1]));
 %!assert (repelem ([1 2 3 4 5], 2,[2 1 2 0 2]), [1 1 2 3 3 5 5;1 1 2 3 3 5 5])
+%!assert (repelem ([1 2 3 4 5], 2,[2 1 2 2 0]), [1 1 2 3 3 4 4;1 1 2 3 3 4 4])
 %
 ## nargin > 3 tests
 %!assert (repelem ([1 0;0 -1], 2, 3, 4), ...
@@ -409,6 +411,7 @@
 %!               -1 -1 0 0 0  1  1  1  1
 %!               -1 -1 0 0 0  1  1  1  1]));
 %!assert (repelem ([1 2 3;4 5 6],[0 2],2,2), repmat([4 4 5 5 6 6],2,1,2))
+%!assert (repelem ([1 2 3;4 5 6],[2 0],2,2), repmat([1 1 2 2 3 3],2,1,2))

 ## test with structures
 %!test


Anonymous

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

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 None (Submitted the item)
  •  

    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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-07-09 rik5 StatusNone Fixed
        Open/ClosedOpen Closed
        Summaryrepelem error if a repeat instruction is a vector ends with 0 repelem error if a repeat instruction is a vector that ends with 0

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code