bugGNU Octave - Bugs: bug #59022, [octave forge] (image) test...

 
 

bug #59022: [octave forge] (image) test failures in blockproc.m under Octave 6.0.90

Submitter:  Hartmut <hardy>
Submitted:  Thu 27 Aug 2020 08:41:36 PM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 6.0.90 Release: 
Operating System:  * GNU/Linux Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 01 Sep 2020 05:21:07 PM UTC, comment #11: 

I reviewed Hartmut patch, split it into two (first change the code to follow current coding guidelines, and the second only fixes this issue), and pushed to default branch. The fix will be art of the next minor release (2.14.0)

Carnë Draug <carandraug>
Group Member
Fri 28 Aug 2020 09:26:24 PM UTC, comment #10: 

Please review the attached PATCH to fix this. It

  • also allows class "inline" for the fun parameter
  • changes all test from using "inline" functions to anonymous functions
  • adds one single test to check of inline functions still work
  • adds plenty of whitespace to improve coding style


I think I would like to continue the (existing) support of blockproc for "inline" functions, as long as they are supported by core Octave. (Why destroy something that already works?)

I think this patch should go to the stable  branch, since it mostly only changes unit tests.

And I think this does not need to trigger a soon "emergency release" of the image package. One (set of) failing unit test(s) under Octave 6.0 will hardly be noticed by anybody. Inline functions won't work any more as inputs to blockproc under the newest Octave versions for a while. But their usage will trigger a warning when used under Octave 6.0. anyways.

Let me know your thoughts to this. Also feel free to adapt, change or push this patch, because I will be offline for a couple weeks soon.

(file #49721)

Hartmut <hardy>
Fri 28 Aug 2020 04:40:13 PM UTC, comment #9: 

I wouldn't check version numbers, but agree with Carnë that using


isa (fun, "inline")


makes sense.

Maybe not supporting (or even removing support for) inline in Toolbox functions is a first step toward removing inline altogether.  But yes, it may be a long time before it is actually removed from Matlab.  See also "isstr".


John W. Eaton <jwe>
Group administrator
Fri 28 Aug 2020 07:50:05 AM UTC, comment #8: 

Matlab returns the following:


>> a = inline('any(x(:))', 'x');
>> class(a)
ans =

    'inline'


so I will guess that the new return value in Octave is Matlab compatible and correct. And while the use of `inline` is not recommended, it is unlikely to ever be removed since it has been around for a very long time and removing it would break too much old code.

I'm with Hartmut in that we should probably just "stick to the "dirty" way with comparing class name strings". Something like this:


  if (! isa (fun,"function_handle")
      && ! isa (fun,"inline function")
      && ! isa (fun,"inline")
      && ! ischar (fun))
    error ("blockproc: invalid FUN parameter.");
  endif


By the way, Matlab's blockproc seems to not accept inline functions in the first place, so I guess we could remove support for them if we want but I see no reason to do so, and would argue that this is a bug in Matlab:


>> blockproc(eye(6),[2,2],inline('any(x(:))','x'))
Error using blockproc>parse_inputs (line 1019)
Invalid block function.  BLOCKPROC expects the user function, FUN, to be a valid function handle.

Error in blockproc (line 226)
[source,fun,options] = parse_inputs(source,block_size,fun,args{:});


Carnë Draug <carandraug>
Group Member
Fri 28 Aug 2020 05:04:10 AM UTC, comment #7: 

Maybe we can solve it in a very dirty way:


v = sscanf (version (), '%d.%d.%d')

if (v(1) < 6)
# current code
    ....
else
# code for version 6 and above
    ...
end


Avinoam Kalma <avinoam>
Group Member
Thu 27 Aug 2020 09:36:01 PM UTC, comment #6: 

What I am still missing is a good way to detect the return object of inline.m in a reliable way under Octave 6 and Octave 5.2 (and earlier Octave versions).

  • typeinfo does not work. It returns "class" under Octave 6.0.90 but "inline function" under Octave 5.2
  • is_function_handle does not work. It returns FALSE under Octave 6.0.90 but TRUE under Octave 5.2.0
  • class does not work (easily). It returns "inline" under Octave 6.0.90 but "function_handle" under Octave 5.2. (and probably returned "inline function" in some earlier Octave version.)


Maybe we need to stick to the "dirty" way with comparing class name strings, and allow both "inline" and "function_handle" and maybe "inline function"?

Hartmut <hardy>
Thu 27 Aug 2020 09:20:24 PM UTC, comment #5: 

The email list comment from comment #4 suggests to use is_function_handle() to check.

But this gives FALSE in Octave 6.0.90 for the return object of inline.m, and it gives TRUE in Octave 5.2.0.

Maybe we should

  • change the 10 test cases to use an anonymous function instead of the inline thing.
  • change the input checking code to do the check via is_function_handle() instead of comparing class names to string values


Doing this would break old user code that uses inline objects with blockproc.m. So maybe this is not the best solution. Any better idea?

Hartmut <hardy>
Thu 27 Aug 2020 09:07:33 PM UTC, comment #4: 
Avinoam Kalma <avinoam>
Group Member
Thu 27 Aug 2020 09:04:20 PM UTC, comment #3: 

I found the following in the help string of Octave's inline.m:


Caution:* the use of 'inline' is discouraged and it may be removed
     from a future version of Octave.  The preferred way to create
     functions from strings is through the use of anonymous functions
     (see Anonymous Functions) or 'str2func'.


Should we exchange the use of "inline" in the problematic test code by an anonymous function, then? An anonymous functions seems to have the classe name "function_handle" which is then properly checked by the code in blockproc.m.

But maybe (to support the use of inline as long as Octave supports it) we should nevertheless additionally change the code of blockproc.m such that it checks for "inline" instead of "inline function" class. Or even allow both strings (since earlier Octave versions would still give "inline function").

Hartmut <hardy>
Thu 27 Aug 2020 08:52:32 PM UTC, comment #2: 

I don't find any occurance of "inline" in the NEWS file of Octave 6.0.90. If this name change is intenional, should we then mention this here?

Hartmut <hardy>
Thu 27 Aug 2020 08:48:45 PM UTC, comment #1: 

In line 102 of blockproc.m we check the following:


  if(!isa(fun,"function_handle") &&
     !isa(fun,"inline function") &&
     !ischar(fun))
    error("blockproc: invalid fun parameter.");
  endif


But the class name seems to have changed from "inline function" to "inline" in Octave 6. At least I see this (in Octave 6):


>> a = inline("sum(x)","x")
a =

  <class inline>

>> class(a)
ans = inline


Is this an intentional name change in Octave 6? Shall we just change the above code line 102 in blockproc.m to check for the new class name "inline" instead of "inline function"?

Hartmut <hardy>
Thu 27 Aug 2020 08:41:36 PM UTC, original submission:  

I have compiled the new Octave release candidate 6.0.90 under (Ubuntu) linux. (All tests passed there). I then de-installed and re-installed the image package (current release 2.12.0) under this Octave 6.0.90 and run the test suite of the image package as well.

One function of the image package gives me 10 FAILs: blockproc.m. (I have not observed any other test failures.)

Here is the output from fntest:

>>>>> processing /home/hartmut/octave/image-2.12.0/blockproc.m
***** assert(blockproc(eye(6),[2,2],"sum"),blockproc(eye(6),[2,2],inline("sum(x)","x")));
!!!!! test failed
blockproc: invalid fun parameter.
***** assert(blockproc(eye(6),[2,2],inline("any(x(:))","x")),eye(3)!=0);
!!!!! test failed
blockproc: invalid fun parameter.
***** assert(blockproc(eye(6),[1,2],[1,1],inline("sum(x(:))","x")),[2,1,0;3,2,0;2,3,1;1,3,2;0,2,3;0,1,2]);
!!!!! test failed
blockproc: invalid fun parameter.
***** assert(blockproc(eye(6),'indexed',[1,2],[1,1],inline("sum(x(:))","x")),[8,5,6;6,2,3;5,3,4;4,3,5;3,2,6;6,5,8]);
!!!!! test failed
blockproc: invalid fun parameter.
***** assert(blockproc(eye(6),[2,3],[4,3],inline("sum(x(:))","x")),ones(3,2)*6);
!!!!! test failed
blockproc: invalid fun parameter.
***** assert(blockproc(eye(6),[2,2],inline("int8(sum(x(:)))","x")),eye(3,"int8")*2);
!!!!! test failed
blockproc: invalid fun parameter.
***** assert(blockproc(uint8(eye(6)),[1,2],[1,1],inline("sum(x(:))","x")),[2,1,0;3,2,0;2,3,1;1,3,2;0,2,3;0,1,2]);
!!!!! test failed
blockproc: invalid fun parameter.
***** assert(blockproc(uint8(eye(6)),'indexed',[1,2],[1,1],inline("sum(x(:))","x")),[2,1,0;3,2,0;2,3,1;1,3,2;0,2,3;0,1,2]);
!!!!! test failed
blockproc: invalid fun parameter.
***** assert(blockproc(uint16(eye(6)),[1,2],[1,1],inline("sum(x(:))","x")),[2,1,0;3,2,0;2,3,1;1,3,2;0,2,3;0,1,2]);
!!!!! test failed
blockproc: invalid fun parameter.
***** assert(blockproc(uint16(eye(6)),'indexed',[1,2],[1,1],inline("sum(x(:))","x")),[2,1,0;3,2,0;2,3,1;1,3,2;0,2,3;0,1,2]);
!!!!! test failed
blockproc: invalid fun parameter.


It seems to me that something has changed in Octave 6 that we have missed before. Is this because of the "inline" function definition, or because of something else?

Hartmut <hardy>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #49721:  blockproc_inline_V1.patch added by hardy (8KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by avinoam (Posted a comment)
  • -email is unavailable- added by hardy (Submitted the item)
  • -email is unavailable- added by hardy
  • -email is unavailable- added by hardy
  •  

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

    Date Changed by Updated Field Previous Value => Replaced by
    2020-09-01 carandraug StatusNone Fixed
        Open/ClosedOpen Closed
    2020-08-28 hardy Attached File- Added blockproc_inline_V1.patch, #49721
    2020-08-27 hardy Carbon-Copy- Added avinoam
        Carbon-Copy- Added carandraug

    Back to the top

    Powered by Savane 3.15-e6e5.
    Corresponding source code