bugGNU Octave - Bugs: bug #58411, cellfun with...

 
 

bug #58411: cellfun with "ErrorHandler" returns wrong message

Submitter:  Muhali <muhali>
Submitted:  Wed 20 May 2020 12:28:54 PM UTC
   
 
Category:  Interpreter Severity:  4 - Important
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 6.0.92 Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 12 Nov 2020 03:06:17 PM UTC, comment #18: 

I think this is a different problem.  The warning() function only prints something if the ID is not empty.  This is Matlab-compatible because a common construct might be


warnmsg = '';
... code which assigns warnmsg only if a problem is found ...
warning (warnmsg);


This produces a warning only when warnmsg is not empty.  The same thing applies to the warning ID as well.

In this case, I modified your ErrorHandler function to test


isempty (S.identifier)


and it is empty so no warning is printed.

This makes sense because if you look at factorial.m the input validation is


    error ("factorial: all N must be real non-negative integers");


Perhaps it would be a good future project to make sure that every warning and every error function call in Octave core had an associated ID, but that will be a lot of work and wouldn't be done for the 6.1 release.

Rik <rik5>
Group administrator
Thu 12 Nov 2020 12:01:31 PM UTC, comment #17: 


comment #16:

> I think we've done enough for this report.  Marking as fixed and closing.


hmm. I'm not sure whether this bug is fixed or a new bug should be filed. Namely, if you slightly modify the code as follows


function varargout = foo (S, varargin)
   warning(S.identifier, S.message) ;
   for i = 1 : nargout
      varargout{i} = NaN ;
   endfor
endfunction
cellfun ("factorial", {1,2,-3}, "ErrorHandler", @foo)


then (again) no warning is issued, unlike in matlab.

Muhali <muhali>
Wed 11 Nov 2020 10:56:32 PM UTC, comment #16: 

I think we've done enough for this report.  Marking as fixed and closing.

Rik <rik5>
Group administrator
Wed 11 Nov 2020 07:55:52 PM UTC, comment #15: 

Rik: Thanks.  I pushed the following changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/4ec8a36990e8

John W. Eaton <jwe>
Group administrator
Mon 09 Nov 2020 07:22:02 PM UTC, comment #14: 

Rik: OK, I hadn't thought about that possibility.  I suppose if uniformoutput is true, then yes, each call should return the same number (and type?) of object.

John W. Eaton <jwe>
Group administrator
Fri 06 Nov 2020 10:51:40 PM UTC, comment #13: 

The trouble with the patch is that some functions may return zero arguments (tmp_len == 0) and this is okay.  For example,


cellfun (@(x) disp (x), {1, 2, 3})
1
2
3


With the patch applied, this aborts early


cellfun (@(x) disp (x), {1, 2, 3})
error: cellfun: function must return value when UniformOutput is TRUE
1


The trick is really that Octave needs to detect a deviation from the expected.  In the factorial example,


cellfun ("factorial", {1,2,-3}, "ErrorHandler", @cellfoo)


nargout is 0, but factorial is expected to produce 1 output, which it does for the first two elements and then the ErrorHandler function does not produce an output and this should cause an error.

In the disp() example, there should be no outputs but if one output were returned that would be an error.

One possibility might be to have an expected_nargout variable that is initialized after calling the function for the first time.  After that the comparison would be between the expected_nargout and the actual number of arguments from get_output_list.

I've attached a diff for that.


(file #50218)

Rik <rik5>
Group administrator
Fri 06 Nov 2020 06:40:51 PM UTC, comment #12: 

How about the attached change?

Do we also need to check for undefined elements in the octave_value_list object returned from get_output_list?

(file #50216)

John W. Eaton <jwe>
Group administrator
Fri 06 Nov 2020 05:52:14 PM UTC, comment #11: 

Rik: Yeah, because nargout is zero, so varargout remains empty.  Apparently Matlab's cellfun detects that situation.  I think it should be fairly simple to fix Octave to do the same.

John W. Eaton <jwe>
Group administrator
Fri 06 Nov 2020 05:50:00 PM UTC, comment #10: 


comment #9:

> Are you running Linux?  What version of g++ are you using?


yes, self-built with Manjaro Linux and g++ (GCC) 10.2.0.

I tested your version (which is my original I guess) and get what I had reported.

Muhali <muhali>
Fri 06 Nov 2020 05:35:54 PM UTC, comment #9: 

@Muhali: Are you using a self-built version of Octave based on hg id e3bc897115ed or did you download a pre-built version of Octave?

Are you running Linux?  What version of g++ are you using?

I run things using a file cellfoo.m and a file tst_cellfun_nan.m which are attached.

cellfoo.m is


function varargout = cellfoo (S, varargin)
   S.message
   for i = 1 : nargout
      varargout{i} = NaN ;
   endfor
endfunction


tst_cellfun_nan.m is


cellfun ("factorial", {1,2,-3}, "ErrorHandler", @cellfoo)


With these files, I definitely get that the third output is initialized to 1.

Rik <rik5>
Group administrator
Fri 06 Nov 2020 05:06:37 PM UTC, comment #8: 

I see this:

ans = factorial: all N must be real non-negative integers
ans =

    1.0000e+00    2.0000e+00   6.9001e-310

with an installation of today

GNU Octave Version: 6.0.93 (hg id: e3bc897115ed)

Muhali <muhali>
Thu 05 Nov 2020 10:24:50 PM UTC, comment #7: 

The original test code still doesn't produce Matlab behavior, but maybe that is okay as the regression (disappearance of error message) is fixed.

Specifically, this code


cellfun ("factorial", {1,2,-3}, "ErrorHandler", @foo)


used to produce an error message and this numerical result


ans =

    1.0000e+00    2.0000e+00   2.5233e-310


It now produces this numerical result


ans =

   1   2   1


In Matlab, it produces the error


Output was expected but not returned while using Uniform output, at index 3. Set ‘UniformOutput’ to false.


Do we want to leave this bug report open and change the Item Group to "Matlab Compatibility" or should this be a new report.  My only reason for continuing here is that there is already sample code and some interested parties.


Rik <rik5>
Group administrator
Thu 05 Nov 2020 08:02:22 PM UTC, comment #6: 

Muhali: Thanks for the info.  Tagging this report as ready for test.

John W. Eaton <jwe>
Group administrator
Thu 05 Nov 2020 06:29:35 PM UTC, comment #5: 


comment #4:

> BTW, in Matlab, what is the value of nargout inside the error handler function?


I get nargout = 0

Muhali <muhali>
Thu 05 Nov 2020 05:42:38 PM UTC, comment #4: 

I pushed the following change to stable and merged with default:

http://hg.savannah.gnu.org/hgweb/octave/rev/9a84aa8c4552

BTW, in Matlab, what is the value of nargout inside the error handler function?

John W. Eaton <jwe>
Group administrator
Mon 26 Oct 2020 08:18:18 AM UTC, comment #3: 

Reviewed and still an issue with Octave 6.0.92.

Kai Torben Ohlhus <siko1056>
Group Member
Mon 01 Jun 2020 10:36:56 PM UTC, comment #2: 

This bug has two parts.  The fact that the last value has nonsense in it, rather than NaN, is due to the "ErrorHandler" routine itself.  The function "foo" cycles over the range 1:nargout.  However, when the calling form is


cellfun (...)


nargout is equal to 0 so no value is assigned by the ErrorHandler.  This is why the invocation


x = cellfun (...)


works because nargout is equal to 1 in this case and the for loop works correctly.

However, what should happen, according to Matlab, is that an error gets thrown because the function is supposed to supply a value.  Matlab emits an error that says that "UniformOutput" is true (default) but the function failed to produce a value for index 3.

By setting "UniformOutput" to false, both Matlab and Octave produce the correct result


cellfun (@factorial, {1,2,-3}, "ErrorHandler", @cellfoo, 'uniformoutput', false)



Rik <rik5>
Group administrator
Wed 20 May 2020 03:34:14 PM UTC, comment #1: 

Confirmed.  This is a regression from 5.2.  The behavior is the same on the development branch as well.

The second part is interesting.  To be clear, the output value is incorrect when assigned to the default variable ("ans").  But is correct if there is an actual assignment to a variable.

Rik <rik5>
Group administrator
Wed 20 May 2020 12:28:54 PM UTC, original submission:  

in continuation of bug #58280: the code


function varargout = foo (S, varargin)
   S.message
   for i = 1 : nargout
      varargout{i} = NaN ;
   endfor
endfunction
cellfun ("factorial", {1,2,-3}, "ErrorHandler", @foo)


returns with ovtave-5

ans = factorial: all N must be real non-negative integers
ans =

    1.0000e+00    2.0000e+00   2.5233e-310


whereas octave-6 returns

ans =
ans =

    1.0000e+00    2.0000e+00   2.5233e-310


so the message is gone.

Moreover, the output of cellfun is only correct if I execute


fac = cellfun ("factorial", {1,2,-3}, "ErrorHandler", @foo)


which returns

fac =

     1     2   NaN

Muhali <muhali>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #50219:  cellfoo.m added by rik5 (187B - text/x-matlab)
file #50220:  tst_cellfun_nan.m added by rik5 (58B - text/x-matlab)
file #50218:  58411.diff added by rik5 (1KiB - text/x-patch)
file #50216:  cellfun-output-diff.txt added by jwe (676B - text/plain)

 

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 siko1056 (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by muhali (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 13 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-11-11 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2020-11-06 rik5 Attached File- Added cellfoo.m, #50219
        Attached File- Added tst_cellfun_nan.m, #50220
    2020-11-06 rik5 Attached File- Added 58411.diff, #50218
    2020-11-06 jwe Attached File- Added cellfun-output-diff.txt, #50216
    2020-11-05 jwe StatusConfirmed Ready For Test
    2020-10-26 siko1056 CategoryOctave Function Interpreter
        Severity3 - Normal 4 - Important
        Release6.0.90 6.0.92
    2020-05-20 rik5 CategoryNone Octave Function
        Item GroupNone Regression
        StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code