bugGNU Octave - Bugs: bug #53185, Range data type needs nnz...

 
 

bug #53185: Range data type needs nnz implementation

Submitter:  Luis Mendo <lmendo>
Submitted:  Sun 18 Feb 2018 06:41:39 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Luis Mendo Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Sun 11 Mar 2018 09:28:14 PM UTC, comment #3: 

I changed the BIST tests slightly because eps(-2) is the same as eps(2).  To test a small delta on eather side of -2 requires -eps(2) and +eps(2).

It's all checked in here http://hg.savannah.gnu.org/hgweb/octave/rev/00ecff875f8a.

@maors: Thanks for your speedy work, both in fixing it and reacting to my comments.

If you're looking for another small piece of work, I think this bug would be easy to squash: https://savannah.gnu.org/bugs/?func=detailitem&item_id=53232.

Marking this report as fixed and closing.

Rik <rik5>
Group administrator
Thu 08 Mar 2018 07:32:00 PM UTC, comment #2: 

@Maor: This is looking good, but I have a few more requests.

First, can you configure your editor to use spaces rather than tabs?  Tab alignment always produces problems because people set the tab expansion to different values.  It is an Octave coding convention to use spaces only for indentation.

Second, I would like the Range data type to support nnz.  This means moving the algorithm you coded from libinterp/octave-value/ov-range.[h|cc] in to liboctave/array/Range.[h|cc].  In this way, people who write their own C++ code and link against liboctave will have access to this function.  The octave_value class is only known by Octave's interpreter.  There will then be a little bit of effort to connect the octave_value nnz function to the Range data type function in ov-range.h, but it should be as simple as this:


  octave_value nnz (void) const { return range.nnz (); }


Also, it would be good to write some BIST tests and place them in data.cc below the definition of nnz.

Some simple examples


/*
%!assert (nnz (1:5), 5)
%!assert (nnz (-5:-1), 5)
%!assert (nnz (-5:5), 10)
*/


And some harder ones


%!assert (nnz (1:5) * 0, 0)
%!assert (nnz (-2+eps(-2):1:2), 5)


Incidentally, the last test is failing.  The number of non-zero elements is 5, but is reported as 4


octave:26> nnz (-2+eps(-2):1:2)
ans =  4
octave:27> x = -2+eps(-2):1:2
x =

   -2.0000   -1.0000    0.0000    1.0000    2.0000

octave:28> x != 0
ans =

  1  1  1  1  1



Rik <rik5>
Group administrator
Mon 19 Feb 2018 01:11:57 AM UTC, comment #1: 

Confirmed.  I also agree with your diagnosis; creating a column vector will conver the special Range octave-value type to an ordinary Matrix type which will then function correctly.

Range is a special type and so there probably needs to be an nnz() overloaded function written for this type.

Rik <rik5>
Group administrator
Sun 18 Feb 2018 06:41:39 PM UTC, original submission:  

The nnz function gives an error whrn the input is a range. For example:


x = 1:5;
nnz(x)


This is probably related to Octave treating ranges specially to save memory, as indicated here

https://www.gnu.org/software/octave/doc/v4.2.0/Ranges.html

The error disappears with


nnz(x(:))


or


nnz(x.')


presumably because indexing or transposing forces the range to be transformed into an actual vector.

Not sure if this affects other functions in addition to nnz.

Thanks to Stack Exchange user @Steadybox for finding this bug.

Luis Mendo <lmendo>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43497:  53185.cset added by maors (4KiB - application/octet-stream - Fix for bug #53185. Fixed the error in the implementaion and moved nnz to the Range class. Also added some tests.)
file #43470:  53185.cset added by maors (2KiB - application/octet-stream - Fix for bug #53185. Implemented nnz for range type.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by maors (Updated the item)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by lmendo (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-03-11 rik5 StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2018-03-09 maors Attached File- Added 53185.cset, #43497
    2018-03-08 rik5 StatusConfirmed In Progress
        Release4.2.1 dev
    2018-03-06 maors Attached File- Added 53185.cset, #43470
    2018-02-27 rik5 Summarynnz errors with range input Range data type needs nnz implementation
    2018-02-19 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code