bugGNU Octave - Bugs: bug #65011, improve input handling for...

 
 

bug #65011: improve input handling for colormaps

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Mon 11 Dec 2023 04:04:22 PM UTC
   
 
Category:  Octave Function Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  10.1.0 (current default) Planned Release:  10.1.0 (current default)
* Mandatory Fields

Add a New Comment Rich Markup
   

Fri 12 Jan 2024 06:08:43 AM UTC, comment #2: 

I changed the input validation to be tighter and updated the BIST tests in this changeset: https://hg.savannah.gnu.org/hgweb/octave/rev/d8e5e55c3cf5.

Marking as Fixed and closing report.

Rik <rik5>
Group administrator
Mon 11 Dec 2023 07:55:11 PM UTC, comment #1: 

I agree.  Octave should avoid Garbage In / Garbage Out calculations by checking its inputs carefully.  In general, we do a better job than Matlab, but there are a lot of m-files in the scripts/ directory and not everything checks its inputs so carefully.

Of interest, I tried real numbers, arrays, and complex numbers with spring() in Matlab and it produces an error, but not during input validation.  Rather, it is during the running of the script and the message is rather opaque.  Matlab does accept negative integers and returns an empty matrix.  I think we need to preserve that compatibility.

Hence, I would change the documentation to say a scalar integer.  And I would change the tests to check for


if (isscalar (n) && isreal (n) && n == fix (n))


As a bonus, the isreal(n) check will also weed out cell arrays or structs which aren't valid input.  Those are accepted by Matlab, but then produce an obtuse error.

Rik <rik5>
Group administrator
Mon 11 Dec 2023 04:04:22 PM UTC, original submission:  

most colormaps, (e.g., viridis(), jet(), etc) take a single numeric input for the number of colors/rows to return. This is specified in the docs as a scalar input.  matlab colormap docs specify that it must be a nonnegative, scalar integer.

Checking the input handling, there is inconsistency between color maps, with only isscalar being checked and the output highly dependent on whether the map is generated from an algorithm, interpolated, etc.

I haven't checked all of the maps listed in 'help colormap', but a few examples:

viridis:

>> viridis (5)
ans =

   2.6700e-01   4.8743e-03   3.2942e-01
   2.3022e-01   3.2130e-01   5.4549e-01
   1.2815e-01   5.6511e-01   5.5089e-01
   3.6286e-01   7.8670e-01   3.8659e-01
   9.9325e-01   9.0616e-01   1.4394e-01

>> viridis (5.1)
ans =

   2.6700e-01   4.8743e-03   3.2942e-01
   2.3022e-01   3.2130e-01   5.4549e-01
   1.2815e-01   5.6511e-01   5.5089e-01
   3.6286e-01   7.8670e-01   3.8659e-01
   9.9325e-01   9.0616e-01   1.4394e-01

>> viridis ([5 4])
error: viridis: N must be a scalar
error: called from
    viridis at line 45 column 7
>> viridis (5+i)
ans =

   2.6700e-01   4.8743e-03   3.2942e-01
   2.3022e-01   3.2130e-01   5.4549e-01
   1.2815e-01   5.6511e-01   5.5089e-01
   3.6286e-01   7.8670e-01   3.8659e-01
   9.9325e-01   9.0616e-01   1.4394e-01

>> viridis(-5)
ans = [](0x3)


spring:

>> spring(5)
ans =

   1.0000        0   1.0000
   1.0000   0.2500   0.7500
   1.0000   0.5000   0.5000
   1.0000   0.7500   0.2500
   1.0000   1.0000        0

>> spring(5.1)
error: conversion of 5.1 to int64_t value failed
error: called from
    spring at line 55 column 7
>> spring([5 4])
error: spring: N must be a scalar
error: called from
    spring at line 40 column 7
>> spring(5+i)
warning: imaginary part of complex colon arguments is ignored
warning: called from
    spring at line 56 column 7

ans =

   1.0000 +      0i        0 +      0i   1.0000 -      0i
   1.0000 +      0i   0.2353 - 0.0588i   0.7647 + 0.0588i
   1.0000 +      0i   0.4706 - 0.1176i   0.5294 + 0.1176i
   1.0000 +      0i   0.7059 - 0.1765i   0.2941 + 0.1765i
   1.0000 +      0i   0.9412 - 0.2353i   0.0588 + 0.2353i

>> spring(-5)
ans = [](0x3)


jet:

>> jet(5)
ans =

        0   0.5000   1.0000
        0   1.0000   1.0000
   0.5000   1.0000   0.5000
   1.0000   1.0000        0
   1.0000   0.5000        0

>> jet(5.1)
error: conversion of 5.1 to int64_t value failed
error: called from
    jet at line 71 column 7
>> jet([5 4])
error: jet: N must be a scalar
error: called from
    jet at line 46 column 7
>> jet(5+i)
error: mod: not defined for complex numbers
error: called from
    jet at line 65 column 20
>> jet(-5)
ans = [](0x3)


(this report follows on patch #10064 proposed for the Spring colormap, but it would make sense to apply any changes to all maps consistently.)

Unless there's a rational use case for allowing non-positive integer valued inputs, I'd recommend adding input checks to catch and provide consistent error messages for other inputs.

Nicholas Jankowski <nrjank>
Group Member

 

(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 nrjank (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-01-12 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 10.1.0 (current default)
    2023-12-11 rik5 Planned ReleaseNone 10.1.0 (current default)

    Back to the top

    Powered by Savane 3.13-8ccc.
    Corresponding source code