bugGNU Octave - Bugs: bug #66642, cellfun & arrayfun return zero...

 
 

bug #66642: cellfun & arrayfun return zero values if function returns fewer outputs than expected

Submitter:  Fernando <tutissanalio>
Submitted:  Mon 06 Jan 2025 11:28:26 AM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Missed Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * stable Release: 
Operating System:  * Any Fixed Release:  11.1.0 (current default)
Planned Release:  11.1.0 (current default)
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 29 May 2025 09:58:00 PM UTC, comment #24: 

Typo in my previous comment. "It should now all be merged away" not "not".

Arun Giridhar <arungiridhar>
Group Member
Thu 29 May 2025 09:57:03 PM UTC, comment #23: 

I backed out 51b86922ebed. It looked identical to 911c6b05f5dc but because of subsequent changes in a4e0e2688c42 it was causing a merge conflict. It should not all be merged away. Pls check if something specific needs to be re-added to the bytecode-interpreter branch.

Arun Giridhar <arungiridhar>
Group Member
Thu 29 May 2025 09:25:43 PM UTC, comment #22: 

I am getting a merge conflict. What is changeset 34596 in this list, and can it be backed out before merging default to bytecode-interpreter?


@  changeset:   34598:a4e0e2688c42
|  tag:         tip
|  user:        Rik <rik@octave.org>
|  date:        Thu May 29 13:36:37 2025 -0700
|  summary:     Clean up code in cset 911c6b05f5dc.
|
o  changeset:   34597:911c6b05f5dc
|  parent:      34594:5e65b3f41100
|  user:        Fernando Alvarruiz <feralber@upvnet.upv.es>
|  date:        Wed May 21 21:04:43 2025 +0200
|  summary:     Check for undefined outputs in cellfun & arrayfun when nargout=0 (bug#66642).
|
| @  changeset:   34596:51b86922ebed
| |  branch:      bytecode-interpreter
| |  user:        Fernando Alvarruiz <feralber@upvnet.upv.es>
| |  date:        Wed May 21 21:04:43 2025 +0200
| |  summary:     Check for undefined outputs in cellfun & arrayfun when nargout=0 (bug#66642).
| |
| o  changeset:   34595:9c8843bfc306
|/|  branch:      bytecode-interpreter
| |  parent:      34579:4be0701ec15b
| |  parent:      34594:5e65b3f41100
| |  user:        Arun Giridhar <arungiridhar@gmail.com>
| |  date:        Thu May 29 16:09:43 2025 -0400
| |  summary:     maint: Merge default to bytecode-interpreter


Arun Giridhar <arungiridhar>
Group Member
Thu 29 May 2025 08:45:20 PM UTC, comment #21: 

I checked in 1) your BIST patch, 2) your patch for greater Matlab compatibility when nargout=0, 3) my own patch to use more Octave coding conventions in the first two changesets.  See

https://hg.savannah.gnu.org/hgweb/octave/rev/2132c73647ac
https://hg.savannah.gnu.org/hgweb/octave/rev/911c6b05f5dc
https://hg.savannah.gnu.org/hgweb/octave/rev/a4e0e2688c42

You are correct that Farrayfun is old code and has not been re-written as I did for Fcellfun.  Evenutally this will happen.  In the meantime, the compiler gave me a warning about the variable "expected_nargout" in Farrayfun possibly being used without initialization.  I initialized the variable to 0, but this should go away entirely after a re-write.

I think enough work has been done on this particular bug report.  Marking bug as Fixed and closing.

Rik <rik5>
Group administrator
Wed 21 May 2025 07:26:35 PM UTC, comment #20: 

Here is a patch which mimics Matlab's behavior when nargout=0. It's a patch to be applied on top of the patch with the BISTs (comment #16).

I think what Matlab does when nargout=0 makes sense: when you call a function directly, with nargout=0, it's OK if the function returns something, and it's also OK if it doesn't. If the call is made via cellfun or arrayfun and UniformOutput=false, you get the same. If UniformOutput=true, all the calls must return something, or none of them should, otherwise the result is not uniform.

Something that strikes me is the fact that the code for Farrayfun is not very similar to the code for Fcellfun, even though they appear to do basically the same things. It seems that both codes could be almost identical, which would simplify maintenance.

(file #57234)

Fernando <tutissanalio>
Sat 17 May 2025 05:31:13 PM UTC, comment #19: 

Going back to this, I think the current code is fine, and it's probably not a good idea to restructure the code in the way suggested in my previous comment. So, the patch with tests is ready to be reviewed.

On the other hand, I want to work on a fix for the issue of comment #16 and comment #17. I think that can also be discussed in this bug report, because it's a particular case (when nargout=0) of the problem stated in the summary.

Fernando <tutissanalio>
Thu 01 May 2025 11:22:19 AM UTC, comment #18: 

Regardles of the issues pointed in the last two comments, I think there could be a better way to handle the problem of the function failing to return an output.

I think that the code is more complex than necessary because the function call returns all the function outputs, even if some of them are undefined. It should be possible to change this, so that the function call does not return the last outputs if they are undefined. However, this would be a more substantial change, potentially affecting other areas.

Fernando <tutissanalio>
Wed 30 Apr 2025 04:10:06 PM UTC, comment #17: 

curious what matlab does:

matlab 2024b:


>> cellfun(@f,{1,-2})
Error using cellfun
Output was expected but not returned while using Uniform output, at index 2.
Set 'UniformOutput' to false.

>> a = cellfun(@f, {1,-2})
Output argument "r" (and possibly others) not assigned a value in the
execution with "f" function.

>> cellfun(@f, {-1,2})
Error using cellfun
Unexpected output was returned while using Uniform output, at index 2. Set
'UniformOutput' to false.

>> a = cellfun(@f, {-1,2})
Output argument "r" (and possibly others) not assigned a value in the
execution with "f" function.

>> cellfun(@f, {-1,2}, "UniformOutput", false)

ans =

  1×2 cell array

    {0×0 double}    {[2]}

>> cellfun(@f, {1,-2}, "UniformOutput", false)

ans =

  1×2 cell array

    {[1]}    {0×0 double}

>> a = cellfun(@f, {-1,2}, "UniformOutput", false)
Output argument "r" (and possibly others) not assigned a value in the
execution with "f" function.

>> a = cellfun(@f, {1,-2}, "UniformOutput", false)
Output argument "r" (and possibly others) not assigned a value in the
execution with "f" function.


Nicholas Jankowski <nrjank>
Group Member
Wed 23 Apr 2025 05:05:59 PM UTC, comment #16: 

I am attaching a patch with some BIST tests. They cover the 6 new error messages introduced, 3 for cellfun:
- cellfun, uniform output, first function call produces fewer outputs than requested.
- the same, but not first function call.
- cellfun, non-uniform output.
and the corresponding 3 for arrayfun.

I also added two tests (one for cellfun and another one for arrayfun) considering the case when the provided function returns MORE outputs than requested, which is correct (extra outputs are ignored).

I also noticed a strange behaviour when calling cellfun (or arrayfun) without output arguments and one of the function calls does not return anything. E.g.:

octave:> function [r]=f(x); if x>0 r=x; end; end
octave:> cellfun(@f,{1,-2})
ans =
   1   0


I would expect the same error as in this case:

octave:> a=cellfun(@f,{1,-2})
error: cellfun: function returned fewer than nargout values


Also, in this other case, with the same f, you get a warning that you reach a code that should be unrechable:

octave:> cellfun(@f,{-1,2})
warning: cellfun: This code should be unreachable.  Please report to bugs.octave.org
ans =
   0   2


I would also expect the same error as before.


(file #57164)

Fernando <tutissanalio>
Wed 16 Apr 2025 04:39:16 PM UTC, comment #15: 

Yes, I still want to write some BIST tests. I've been rather busy but I think I will have some time in the next week or so. Thanks for the info on the rewrite.

Fernando <tutissanalio>
Wed 16 Apr 2025 04:13:30 PM UTC, comment #14: 

@Fernando: Just a friendly reminder, were you going to write some BIST tests for this bug report?  I re-wrote a lot of cellfun.cc yesterday so you should look at the code on the default branch of Mercurial.

Rik <rik5>
Group administrator
Tue 14 Jan 2025 10:58:38 PM UTC, comment #13: 

I checked in your changeset here: https://hg.savannah.gnu.org/hgweb/octave/rev/2c2301104caf.

In addition, I switched to using the function "is_undefined ()" for clarity over "! is_defined ()".  See https://hg.savannah.gnu.org/hgweb/octave/rev/00671f5dbc55.

The only thing left is to write some BIST tests to verify that the behavior is correct and to catch anything if the code is modified in the future.  Marking the bug as "In Progress".

Rik <rik5>
Group administrator
Sun 12 Jan 2025 05:00:06 PM UTC, comment #12: 

Yes, I'd like to do it.

Fernando <tutissanalio>
Sun 12 Jan 2025 11:08:40 AM UTC, comment #11: 

Thank you for tracking this down and providing a patch.
Your suggested changes look reasonable to me.

Would you like to add some tests (ideally one for each new error message) to make sure that this doesn't accidentally regress in the future?

Markus Mützel <mmuetzel>
Group administrator
Sat 11 Jan 2025 02:03:20 PM UTC, comment #10: 

Ok, I see what you mean. In the case of UniformOutput==true, current code (in stable) for cellfun only copies a function output if the ouput is defined. If it's undefined, it doesn't do the assignment. That's why you don't get the error.

This is done by the block of code starting on line 581 of cellfun.cc:


                  for (int j = 0; j < num_to_copy; j++)
                    {
                      if (tmp(j).is_defined ())
                        {
                          octave_value val = tmp(j);

                          if (val.numel () != 1)
                            error ("cellfun: all values must be scalars when UniformOutput = true");

                          retv[j] = val.resize (fdims);
                        }
                    }


and also at a similar block starting at line 596.

In the case of non-uniform output, Octave assigns the outputs in lines 663-664:


              for (int j = 0; j < num_to_copy; j++)
                results[j](count) = tmp(j);


which will assign the value, even if undefined, to a cell element. I guess the assignment operation does not check if the value being assinged is defined. I don't know if it should, or if we should take care not to assign undefined elements.

Fernando <tutissanalio>
Fri 10 Jan 2025 05:42:47 PM UTC, comment #9: 

right, so maybe I'm still not understanding why the call to the f within cellfun/arrayfun doesn't generate the same error.  isn't there an attempted assignment missing a RHS value?

Nicholas Jankowski <nrjank>
Group Member
Fri 10 Jan 2025 05:17:09 PM UTC, comment #8: 

comment #7:

> It would be useful to me to better understand why the call to f within cellfun fails to generate the error that occurs when it's called alone, and whether something is being missed there that should be addressed.


When function f is called alone, the error is generated by the instruction where f is called. E.g. if we have a multi-assignment:
[a,b]=f(x)
the error happens when copying the function output to the lvalues a, b. This is in pt-assign.cc (tree_multi_assignment::evaluate_n), line 291:

                      octave_value tmp = rhs_val(k);

                      if (tmp.is_undefined ())
                        error ("element number %" OCTAVE_IDX_TYPE_FORMAT
                               " undefined in return list", k+1);


If the instruction is a simple assignment, the error is generated in pt-assign.cc (tree_simple_assignment::evaluate), line 103:

          if (rhs_val.is_undefined ())
            error ("value on right hand side of assignment is undefined");


So, this is done case by case, depending on the instruction. That's why we have to consider that also in cellfun and arrayfun.

Fernando <tutissanalio>
Wed 08 Jan 2025 09:28:29 PM UTC, comment #7: 

it may be worth working up a few more BISTs to tease out and verify a few more things like this. (current bists only test uniformoutput=true)

initially i was concerned this was more of an interpreter level issue that should also catch issues with cellfun, but for arbitrary function inputs i guess it makes sense that the interpreter won't be able to tell the difference as long as we're pre-populating the outputs.

It would be useful to me to better understand why the call to f within cellfun fails to generate the error that occurs when it's called alone, and whether something is being missed there that should be addressed.

Nicholas Jankowski <nrjank>
Group Member
Wed 08 Jan 2025 08:21:33 PM UTC, comment #6: 

There's also another case solved by the patch. It's the following:


octave:> function [r]=f(x); if(x>0) r=x ;end; end
octave:> a=f(3)
      a = 3
octave:> a=f(-3)
error: value on right hand side of assignment is undefined
octave:> % everything's fine up to now
octave:> a=cellfun(@f,{1,-2,3},"uniformoutput",false)
      a =
      {
        [1,1] = 1
        [1,2] =

error: octave_base_value::print (): wrong type argument '<unknown type>'


and you end up with a cell array (a) with an undefined element.

Fernando <tutissanalio>
Wed 08 Jan 2025 07:55:16 PM UTC, comment #5: 

I've been having a look at cellfun.cc. The problem is that you get all outputs expected, but some outputs are undefined. I attach a possible patch.

(file #56750)

Fernando <tutissanalio>
Wed 08 Jan 2025 04:34:38 PM UTC, comment #4: 

a simple check of inserting an error() line into f shows that cellfun does properly stop for errors in the function.

also, arrayfun shows the same behavior as cellfun.  updating title and added another BIST as
https://hg.savannah.gnu.org/hgweb/octave/rev/f901d8fd3091

Nicholas Jankowski <nrjank>
Group Member
Tue 07 Jan 2025 05:21:51 PM UTC, comment #3: 

i pushed a BIST to capture the expected error as:
https://hg.savannah.gnu.org/hgweb/octave/rev/6b0b6c049ac1

Nicholas Jankowski <nrjank>
Group Member
Mon 06 Jan 2025 04:17:36 PM UTC, comment #2: 

i haven't looked through cellfun before, but there does appear to be an attempt to catch a mismatch in defined number of return variables. Starting with this block at Line 558:


          const octave_value_list tmp
            = get_output_list (interp, count, nargout, inputlist, fcn,
                               error_handler);
          int tmp_numel = tmp.length ();
          if (count == 0)
            expected_nargout = tmp_numel;
          else if (tmp_numel != expected_nargout)
            error ("cellfun: function returned unexpected number of values");

          if (nargout > 0 && tmp_numel < nargout)
            error ("cellfun: function returned fewer than nargout values");


So this check passes because f is defined with 2 outputs, which matches the cellfun call for 2 outputs.

Why the first actual call to f doesnt trigger the error is the question then.

Nicholas Jankowski <nrjank>
Group Member
Mon 06 Jan 2025 03:33:15 PM UTC, comment #1: 

Confirming report on stable.  Matlab also produces an error so this is not a compatibility issue.

matlab 2024b:

>> f(1)

ans =

     1

>> [a,b] = f(1)
Output argument "b" (and possibly others) not
assigned a value in the execution with "f"
function.

>> [a,b] = cellfun (@f, {1,4})
Output argument "b" (and possibly others) not
assigned a value in the execution with "f"
function.


my guess is that cellfun is prepopulating the output arrays and fails to catch lack of content for any outputs.

Nicholas Jankowski <nrjank>
Group Member
Mon 06 Jan 2025 11:28:26 AM UTC, original submission:  

In current stable and default branches, cellfun incorrectly assigns a value of zero for values not returned by the called function, when the function returns fewer outputs than declared.


octave:> ver octave
----------------------------------------------------------------------
GNU Octave Version: 9.3.1 (hg id: 81462b707b37)
GNU Octave License: GNU General Public License
Operating System: Linux 6.8.0-51-generic #52-Ubuntu SMP PREEMPT_DYNAMIC Thu Dec  5 13:09:44 UTC 2024 x86_64
----------------------------------------------------------------------
octave:> function [a,b]=f(x);a=x;end
octave:> s = struct ("a", 1, "b", 4);
octave:> [a,b]=cellfun(@f,{1,4})
a =
   1   4
b =
   0   0


Instead, I would expect an error, like the one you get when calling the function directly:


octave:> [a,b]=f(1)
a = 1
error: element number 2 undefined in return list


Fernando <tutissanalio>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #57234:  bug66642-fix2.patch added by tutissanalio (11KiB - text/x-patch)
file #57164:  bist-66642.txt added by tutissanalio (3KiB - text/plain)
file #56750:  bug66642.patch added by tutissanalio (3KiB - 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 arungiridhar (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by tutissanalio (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 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2025-05-29 rik5 Open/ClosedOpen Closed
    2025-05-29 rik5 StatusIn Progress Fixed
        Fixed ReleaseNone 11.1.0 (current default)
        Planned Release10.1.0 11.1.0 (current default)
    2025-05-21 tutissanalio Attached File- Added bug66642-fix2.patch, #57234
    2025-04-23 tutissanalio Attached File- Added bist-66642.txt, #57164
    2025-01-14 rik5 StatusPatch Submitted In Progress
        Planned ReleaseNone 10.1.0
    2025-01-10 nrjank StatusConfirmed Patch Submitted
    2025-01-08 tutissanalio Attached File- Added bug66642.patch, #56750
    2025-01-08 nrjank Summarycellfun assumes zero values when function returns fewer outputs than expected cellfun & arrayfun return zero values if function returns fewer outputs than expected
    2025-01-06 nrjank StatusNone Confirmed

    Back to the top

    Powered by Savane 3.15-e6e5.
    Corresponding source code