bugGNU Octave - Bugs: bug #66086, interp2() function input...

 
 

bug #66086: interp2() function input validation and error messages need improvement

Submitter:  Yu Hongbo <yuhongbo>
Submitted:  Wed 14 Aug 2024 03:15:11 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Ready For Test Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * 9.2.0 Operating System:  * Any
Fixed Release:  None Planned Release:  9.3.0 (current stable)
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 14 Aug 2024 06:21:38 PM UTC, comment #10: 

it is worth pointing out that octave is also not the most consistent in accepting/rejecting logicals for numeric inputs. e.g.,

>> interp1(logical(eye(5)),1:5)
ans =

   1   0   0   0   0
   0   1   0   0   0
   0   0   1   0   0
   0   0   0   1   0
   0   0   0   0   1

>> interp3(logical(cat(3, eye(4), eye(4))))
error: Invalid call to interp3.  Correct usage is:
...


interpn behaves similarly to interp3. interpft also accepts logical inputs. (well, it calls fft(x) which does any x validation, and that likely internally converts to double).

pushed https://hg.savannah.gnu.org/hgweb/octave/rev/3606d14d6317
to stable after verifying it passes a make check. it updates error messages, docstring, and BISTs in interp2, interp3, and interpn to be a bit more consistent. no intended behavior change.  I suspect interp1 input validation is deliberately minimal to reduce overhead as it's likely called rather often by other functions. no real reason to change that unless it becomes an issue.

marking as Ready for Test.

Nicholas Jankowski <nrjank>
Group Member
Wed 14 Aug 2024 03:50:27 PM UTC, comment #9: 

Note that converting logical array to double increases it size 8 times.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Wed 14 Aug 2024 03:03:42 PM UTC, comment #8: 


comment #6:

Ok. I manual convert logical to double.

Yu Hongbo <yuhongbo>
Wed 14 Aug 2024 02:57:02 PM UTC, comment #7: 

i should have a patch to stable ready shortly that will improve the error message and catch the comment #2 case slipping past the Z input validation.

Nicholas Jankowski <nrjank>
Group Member
Wed 14 Aug 2024 02:55:58 PM UTC, comment #6: 

regarding your comment #4 - yes, + double(0) does a type conversion to double.  you can do this on any input before passing to interp and it will interpolate as floating point numbers.

Nicholas Jankowski <nrjank>
Group Member
Wed 14 Aug 2024 02:49:00 PM UTC, comment #5: 

within the context of the octave language, logical is not numeric:

>> isnumeric(true)

ans = 0


Yes, there are type conversion rules. But this is not always done, and where it is done is specific to what makes sense for that case.   From a matlab compatibility standpoint, the isnumeric behavior, and octave's behavior for the interp functions, appears to be consistent.

if you would propose we extend interp handling for class logical, there would need to be a consistent definition of how that handling should occur. 


Nicholas Jankowski <nrjank>
Group Member
Wed 14 Aug 2024 02:46:59 PM UTC, comment #4: 

So, to quick fix this bug, simply add a double(0) is okay, like this:

>> result = [true false; false true]

result =

  1  0
  0  1

>> result + double(0)

ans =

   1   0
   0   1

Yu Hongbo <yuhongbo>
Wed 14 Aug 2024 02:43:26 PM UTC, comment #3: 


comment #1:

> Interp2 accepts only NUMERICAL matrices as input. Numerical in octave means integer, real (single or double), or complex. Logical and character arrays are NOT considered to be numeric.


I don't agree with this. logical data can sometimes auto convert to double, so logical data may be regarded as numerical data.

>> double(1) + true

ans = 2

Yu Hongbo <yuhongbo>
Wed 14 Aug 2024 12:52:46 PM UTC, comment #2: 

yes, recommend changing the report summary to focus on error message / input validation.  stumbled on some other things too:

interp2(1:10)
error: index (1): out of bound 0 (dimensions are 0x0)
error: called from
    interp2 at line 232 column 11

should be a handled error that size must be >=2 in each interpolated dimension.  there may be others.  the docstring could also better articulate function input type/size requirements

Nicholas Jankowski <nrjank>
Group Member
Wed 14 Aug 2024 12:14:57 PM UTC, comment #1: 

Interp2 accepts only NUMERICAL matrices as input. Numerical in octave means integer, real (single or double), or complex. Logical and character arrays are NOT considered to be numeric.
(Matlab does only accept single, double or complex in interp2).

As a general (good) design principle, automatic data type conversions are limited in octave (like in Matlab). See https://docs.octave.org/latest/Automatic-Conversion-of-Data-Types.html.
In fact, usually octave functions try to create the same output data type(s) as the (key) input data type(s) when appropriate and throw errors when input data type(s) are not supported by the function.

The current interp2 error message is not fully exhaustive, though. Error message could be improved by adding the word "numerical":
interp2: Z must be a numerical 2-D matrix.



Hendrik K <koerhen>
Wed 14 Aug 2024 03:15:11 AM UTC, original submission:  

For example, imread() a pbm picture will generate a logical 2-D matrix.
                                                                    

>> input_pbm=imread('input_pbm.pbm');
>> size(input_pbm)

ans =

   270   480

When interp this picture, the real problem is, logical data cannot auto convert to double data, and interp2() function only take double matrix, which is not "error: interp2: Z must be a 2-D matrix".

>> interp2(input_pbm, 2, 'nearest');

error: interp2: Z must be a 2-D matrix
error: called from
    interp2 at line 134 column 5

>> interp2(double(input_pbm), 2, 'nearest');
>>

Yu Hongbo <yuhongbo>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #56354:  input_pbm.pbm added by yuhongbo (16KiB - 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 dasergatskov (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by koerhen (Posted a comment)
  • -email is unavailable- added by yuhongbo (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
    2024-08-14 nrjank StatusIn Progress Ready For Test
    2024-08-14 nrjank Release9.1.0 9.2.0
        Planned ReleaseNone 9.3.0 (current stable)
    2024-08-14 nrjank StatusConfirmed In Progress
    2024-08-14 nrjank StatusNone Confirmed
        Operating SystemMicrosoft Windows Any
        Summaryinterp2() function has a misunderstanding error when interp a logical matrix interp2() function input validation and error messages need improvement
    2024-08-14 yuhongbo Attached File- Added input_pbm.pbm, #56354

    Back to the top

    Powered by Savane 3.13-8a8d.
    Corresponding source code