bugGNU Octave - Bugs: bug #39257, handles to nested functions are...

 
 

bug #39257: handles to nested functions are not yet supported

Submitted by:  None
Submitted on:  Sat 15 Jun 2013 12:08:54 AM UTC  
 
Category:  Interpreter Severity:  4 - Important
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Matthew Moskewicz Originator Email:  -email is unavailable-
Open/Closed:  Closed Release:  dev
Operating System:  GNU/Linux

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Thu 07 Mar 2019 06:54:15 PM UTC, comment #36: 

I added the BIST test from Stack Overflow in comment #34 to the set of fixed tests here (https://hg.savannah.gnu.org/hgweb/octave/rev/7c9a681c4474).

I didn't add the test in comment #35 because the expected value is not known.

Rik <rik5>
Project Administrator
Thu 07 Mar 2019 05:24:43 AM UTC, comment #35: 

What will be the output of the following test case?


function  [fh1, fh2] = counter
  value = 0;
  fh1 = @increment;
  fh2 = @increment;
  output1 = [f1() f1() f2() f1() f2()]
  function currentValue = increment
    value = value+1;
    currentValue = value;
  end


  fh1 = @increment;
  value = 30;
  fh2 = @increment;
  output2 = [f1() f1() f2() f1() f2()]


  fh1 = @increment;
  fh2 = @increment;
  value = 60;
  output3 = [f1() f1() f2() f1() f2()]

  fh1 = @increment;
  fh2 = @increment;

  output4 = [f1() f1() f2() f1() f2()]
  value = 90;
end

[f1 , f2] = counter();

output5 = [f1() f1() f2() f1() f2()]

Anonymous
Thu 07 Mar 2019 05:07:14 AM UTC, comment #34: 

Here is a test case for testing memoization capability of nested functions [1].


function fcnHandle = counter
  value = 0;
  function currentValue = increment
    value = value+1;
    currentValue = value;
  end
  fcnHandle = @increment;
end

f1 = counter();
f2 = counter();
output = [f1() f1() f2() f1() f2()];

It should produce [1 2 1 3 2].

Anonymous
Wed 06 Mar 2019 01:48:40 AM UTC, comment #33: 

Verified that cellfun() is correctly calculated even when an old-style class is present.

Rik <rik5>
Project Administrator
Tue 05 Mar 2019 11:02:32 PM UTC, comment #32: 

As of the following changeset, I believe the problem described in comment #31 has been fixed:

http://hg.savannah.gnu.org/hgweb/octave/rev/8bd9fd99c12a

John W. Eaton <jwe>
Project Administrator
Tue 05 Mar 2019 09:02:21 AM UTC, comment #31: 

It happens if you happen to have a class in your path that also defines an isempty method.

%%%% @cls/cls.m
function obj = cls
obj = class(struct([]),'cls');

%%%% @cls/isempty.m
function tf = isempty(this)
tf = true;

Guillaume <gyom>
Mon 04 Mar 2019 05:57:01 PM UTC, comment #30: 

It seems to be more complicated than that. You are right that the example works in a new Octave session. I still manage to reproduce the error in a more complex setting - I'll try to reduce it to the bare minimum and report back.

Guillaume <gyom>
Mon 04 Mar 2019 03:39:03 PM UTC, comment #29: 

@Guillaume: Your example works for me.  I am at hg rev 10653e92e205.

Are you referencing the patch over at bug #48086?

Rik <rik5>
Project Administrator
Mon 04 Mar 2019 10:19:22 AM UTC, comment #28: 

With the patch, I now see errors with usual function handles:

>> cellfun(@isempty, {1})
error: isempty: no method for class double

Guillaume <gyom>
Sun 03 Mar 2019 11:23:52 PM UTC, comment #27: 

We definitely need more tests for them.  I included some in my patch for basic scoping issues, but I'm sure there are many more things we could test.

John W. Eaton <jwe>
Project Administrator
Sun 03 Mar 2019 10:12:10 PM UTC, comment #26: 

Should there be a new issue report to create BIST tests for handles to nested functions?  Several of the test code examples in this report would probably be useful to put in the fixed test/ directory.

Rik <rik5>
Project Administrator
Sun 03 Mar 2019 09:42:03 PM UTC, comment #25: 

Wow, this bug has been unresolved since 2013!

I bet this actually fixes around 5 bug reports on Savannah as this same issue has been at the root of a lot of incompatibility with Matlab.  It will be a matter of searching for the right terms to find them.

I checked in a changed to NEWS to announce this important change (https://hg.savannah.gnu.org/hgweb/octave/rev/fab67369e802).

The tests from comment #19 and #20 pass for me.  Marking as fixed and closing report.

Rik <rik5>
Project Administrator
Sun 03 Mar 2019 07:12:28 PM UTC, comment #24: 

With the following changeset, I think this feature is mostly working now:

http://hg.savannah.gnu.org/hgweb/octave/rev/6e9034836239

John W. Eaton <jwe>
Project Administrator
Wed 02 May 2018 04:12:08 PM UTC, comment #23: 

Hello,
I am a developer of Manopt (a Matlab toolbox for optimization on manifolds) and have been tracking the status of nested function handles in Octave since 2014. At this point, lack of this feature is the only reason why Manopt is not compatible with Octave. I am sure there are other Matlab packages in the same situation. I am posting this comment here in case it can serve as further motivation for the people who are working hard on bringing this complex feature to Octave.
Thank you all!
Nicolas

Nicolas Boumal <nicolasboumal>
Wed 04 Apr 2018 03:19:24 PM UTC, comment #22: 

In Matlab comment #20 results in

fh1 =
  function_handle with value:
    @foo/nest

fh2 =
  function_handle with value:
    @foo/nest

z =
     1

z =
     2

z =
     2

z =
     3

fh3 =
  function_handle with value:
    @foo/nest

z =
     1

z =
     4

z =
     4

z =
     5

This is the behavior I would expect: all accessing the same persistent memory space.

For comment #19:

ans =
     1     2     4

ans =
     1     2     4

ans =
     1     2     4

ans =
  function_handle with value:
    @foo/nest

ans =
     1     2     4

ans =
     1     2     4

ans =
     1     2     4

This one surprised me, but makes sense after thinking about it. Each @nest function handle points to the same function with the same memory space. That memory space is static after returning from foo(). Calling the middle foo doesn't change the memory space of the prior function. However this:

[fh1, fh2, fh3] = foo (2);
fh1 (1)
fh2 (1)
fh3 (1)
[fh4, fh5, fh6] = foo (20);
fh1 (1)
fh2 (1)
fh3 (1)
fh4 (1)
fh5 (1)
fh6 (1)

results in

ans =
     1     2     4

ans =
     1     2     4

ans =
     1     2     4

ans =
     1     2     4

ans =
     1     2     4

ans =
     1     2     4

ans =
     1    20     4

ans =
     1    20     4

ans =
     1    20     4

This shows that two separate functions calls establish a new memory space (because they are not persistent variable), and the anonymous functions there refer to the memory space of the function in which they were created. At least that is how I understand it.

Marshall <marsian>
Tue 03 Apr 2018 06:09:08 PM UTC, comment #21: 

Hi,
on comment #19:
you are exactly right, the time of creation decides. So each new handle would require a new copy of the (relevant) local workspace.

on comment #20:
This is something I cannot answer from the top of my head and I will test this tomorrow.

Generally nested functions should work the same way as anonymous functions with regard to closures, which work already in Octave as desired. So if you change the nested function in comment #11 (or comment #19) to an anonymous function it works. There similarly a copy of the workspace has to be made, right? If you take your example from comment #20 and change the nested to an anonymous function, also the persistent variable is "frozen" for each call of the enclosing function.

Thanks for looking into this.

CH <atcl>
Tue 03 Apr 2018 03:29:40 PM UTC, comment #20: 

Also, with the following function:

function [fh1, fh2] = foo ()
  persistent z
  z = 1;
  function r = nest (x)
    z
    z = x
  end
  fh1 = @nest;
  fh2 = @nest;
end

What does this do:

[fh1, fh2] = foo ()
fh1 (2)
fh2 (3)
fh3 = foo ()
fh1 (4)
fh3 (5)

John W. Eaton <jwe>
Project Administrator
Tue 03 Apr 2018 03:17:57 PM UTC, comment #19: 

What value of variables from the enclosing scope is supposed to be used?  The value at the time the handle is created?  What about something like this

function [fh1, fh2, fh3] = foo (y)
  fh1 = @nest;
  function r = nest (x)
    r = [x,y,z];
  end
  z = 3;
  fh2 = @nest;
  z = 4;
  fh3 = @nest;
end

Given the above function, what does Matlab do for

[fh1, fh2, fh3] = foo (2);
fh1 (1)
fh2 (1)
fh3 (1)
foo (20)
fh1 (1)
fh2 (1)
fh3 (1)

?

In any case, I suspect that this is too complicated to fix properly for 4.4 and will have to wait until later, so I will likely undo the change I made earlier today.

John W. Eaton <jwe>
Project Administrator
Tue 03 Apr 2018 02:44:42 PM UTC, comment #18: 

Hi,
thanks for working on this. I tested this and got an error on the example in comment #11. running the function testnested works, also a function handle is returned correctly, yet calling the returned handle gives an error:

octave:2> y = testnested;
octave:3> y()
error: 'y' undefined near line 7 column 8
error: called from
    testnested>nested at line 7 column 3

It seems the variable "a" in the enclosing (closure generating) function went out of scope and is destroyed(?). As far as I understand it Matlab stores a copy of the closure generating function's "workspace" to prevent this until all references to the handle are deleted.

CH <atcl>
Tue 03 Apr 2018 12:17:28 PM UTC, comment #17: 

Hmm, maybe this bug is fixed now except for removing the condition and error message in ov-fcn-handle.cc?  I did that and the few simple tests I tried worked.  So I pushed this changeset on stable and merged with default:

  http://hg.savannah.gnu.org/hgweb/octave/rev/daff5efe062f

If there are other test cases that fail, then please file new bug reports about them so they can be added to the test suite and I'll also try to take a look.

John W. Eaton <jwe>
Project Administrator
Thu 29 Mar 2018 04:50:40 AM UTC, comment #16: 

This bug prevent the usage of many external matlab packages.
see also http://lists.gnu.org/archive/html/octave-maintainers/2018-03/msg00268.html

Raising the severity to 4 - important.

Avinoam Kalma <avinoam>
Project Member
Wed 07 Mar 2018 05:04:53 PM UTC, comment #15: 

Adding jwe to the thread since he has done the recent work on the symbol table.  The function do_multi_index_op no longer exists.  The error about handles to nested functions is coming from this subroutine in ov-fcn-handle.cc:

octave_fcn_handle::octave_fcn_handle (const octave_value& f,
                                      const std::string& n)
  : fcn (f), nm (n), has_overloads (false)
{
  octave_user_function *uf = fcn.user_function_value (true);

  if (uf && nm != anonymous)
    {
      octave::symbol_scope uf_scope = uf->scope ();

      if (uf_scope)
        uf_scope.cache_name (nm);
    }

  if (uf && uf->is_nested_function () && ! uf->is_subfunction ())
    error ("handles to nested functions are not yet supported");
}

Rik <rik5>
Project Administrator
Wed 07 Mar 2018 09:08:54 AM UTC, comment #14: 

I am trying to adapt patch v3 to the current state of the code base, yet the function "octave_user_function::do_multi_index_op" in "ov-usr-fcn.cc" seems to have moved or has been renamed. Could somebody point me in the right direction?

CH <atcl>
Thu 16 Nov 2017 10:08:00 AM UTC, comment #13: 

Just a vote to say that I too am very interested in having Octave officially support handles to nested functions.  I frequently use such handles in my libraries and programs and it would be great for my users if they could run my codes under Octave out of the box.  If it would be helpful, I could help test such patches by trying my codes with them.  Tim

Anonymous
Sat 17 Jun 2017 05:51:23 PM UTC, comment #12: 

I have tried to apply the last patch, but it failed.
the log attached.
Please help...

(file #40931)

Avinoam Kalma <avinoam>
Project Member
Fri 28 Apr 2017 07:19:13 AM UTC, comment #11: 

Hi,

I applied the most recent patch set and tested this:

function y = testnested()

a = 1;

function nested()

  disp(a);

end

y = @nested;

end

The code fails with the error message:

"Handles of nested functions cannot be function return values or the value of global variables when a function returns"

This is a severe limitation for me, especially considering I can return handles to anonymous functions. Can this feature be added in a new patch?

Running the nested function inside the host instead of returning a handle works though.

CH <atcl>
Wed 01 Mar 2017 01:24:44 AM UTC, comment #10: 

Hi Lachlan, I'm very keen to use your patch since I have lots of cellfun(@func,cellA) in my codes, where "func" is a nested function defined within a main function containing the cellfun(@func,cellA). But since I'm on Windows and using the latest Octave 4.2.0, could you please let me know how can I patch and recompile it? Thanks in advance!

Random Nobody <testplayer>
Fri 15 Jul 2016 11:59:53 AM UTC, comment #9: 

Here is a refreshed patch, reflecting recent code changes.

(file #37889)

Lachlan Andrew <lachlan>
Tue 16 Feb 2016 10:23:44 PM UTC, comment #8: 

Quentin, the fix requires patching the source and recompiling.

Is there a reason you can't do that, or is it just that you have never done it before?  What OS are you running?  If you have time, I could try to talk you through the steps.

I've also attached an updated patch; the previous one failed when returning non-scalar structures.

(file #36358)

Lachlan Andrew <lachlan>
Tue 16 Feb 2016 02:23:30 PM UTC, comment #7: 

I have been using a lot of handles + nested functions with Matlab in order to build GUIs. It seems that you have found a solution to solve this trouble in Octave.
Is there any way for me to try it or it has to be implemented in the source (thing that I can not do)?

Quentin Juette <qjuette>
Sun 07 Feb 2016 11:00:21 AM UTC, comment #6: 

The attached patch allows a handle to be taken of a nested function, and prevents that handle being exported (either as a return value or through a global variable) out of the scope in which the shared variables are defined.

This does not allow constructs like the one in comment #3.  However, it should allow GUIs made through Guide.  Please report back here if they cause problems.

I plan to check how Matlab handles constructs like comment #3.  Specifically, I want to know if it use the most recent value assigned to x, including subsequent calls to test_closure, or if it sticks with the value of x at the function exit.  In the former case, we could just make  x  persistent.  In the latter case, the handle would somehow have to keep its own copy of x, like an anonymous function does.

(file #36269)

Lachlan Andrew <lachlan>
Sat 06 Feb 2016 12:20:52 PM UTC, comment #5: 

Rik, you wrote in comment #1 that "during the implementation of subfunctions, there was something really troublesome about handles to nested functions".  Do you by any chance remember the trouble (from back in 2013)?

When I remove the explicit prohibition, and try the code in comment #3, it complains that x is undefined.  If that is the problem, I'm interested in exploring solutions.

That problem only occurs if the return value is a handle to a nested function that accesses a variable from a higher scope.  That shouldn't affect the GUIDE-generate code, or code (such as mine) written by tweaking Matlab example code.

If a return value contains such a handle, one approach may be to copy the parse tree and replace all references to those variables from a higher scope by references to local copies of those variables, initialized at the start of each call to have the value at the time the handle was created.

A complication would be if this function called another nested function which referred to different variables from the higher scope.

If this issue of using variables after they go out of scope was the problem, the error could just be replaced by a warning, and people could use the handles for the cases that do work.

Thoughts?

Lachlan Andrew <lachlan>
Thu 09 Jul 2015 02:57:31 PM UTC, comment #4: 

I'd like to add another data point for why it would be nice to have this ability, now that Octave supports nested functions.  I have a large GUI built in Matlab Guide that I'd like to export.  Matlab will put all the figure generation code into one .m file so you don't have to read the .fig file for the GUI.  However, the way Matlab implements the code for the callbacks in Guide GUIs is via calls to nested function handles.

This seems to me to be a fairly large class of Matlab scripts that it would be really nice to be able to run in Octave.

Anonymous
Wed 10 Jun 2015 07:49:31 PM UTC, comment #3: 

I can confirm that the issue is still present in octave 4.0.0.
As a consequence one cannot use nested function to define a closure. The following code would run in matlab but not in octave:

>> addtwo = test_closure(2)

addtwo =

    @(a)innerfun(a)

>> addtwo(3)

ans =

     5

where test_closure.m is defined as:

function f = test_closure(x)

f = @innerfun; % fails in octave
%f = @(a) innerfun(a); % fails too in octave
%f = @(a) a + x; % only this works in octave

function s = innerfun(a)
  s = a + x;
end
end

Alexander Barth <abarth>
Mon 24 Jun 2013 01:01:29 AM UTC, comment #2: 

Marking the issue as confirmed.  I also just happened to have found a use for handles to nested functions in one of the core Octave m-files.  I had to work around it too.

Rik <rik5>
Project Administrator
Thu 20 Jun 2013 03:21:35 AM UTC, comment #1: 

From what I remember during the implementation of subfunctions, there was something really troublesome about handles to nested functions and they were therefore specifically disallowed.  Unfortunately, this probable means that this issue won't get fixed soon.

Rik <rik5>
Project Administrator
Sat 15 Jun 2013 12:08:54 AM UTC, original submission:  

as per:
http://hg.savannah.gnu.org/hgweb/octave/rev/be18c9e359bf

nested functions with scope rules compatible with matlab are now supported. however, also as per the patch, it appears that handles to nested functions are not yet supported. i didn't find any discussion of this issue or an existing open bug.

i ran into this limitation when trying to run some computer vision related code from here with octave 3.7.5:
http://www.vision.caltech.edu/Image_Datasets/CaltechPedestrians/
code archive:
http://www.vision.caltech.edu/Image_Datasets/CaltechPedestrians/files/code3.0.1.zip

the file dbBrowser.m uses many references to subfunctions.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #40931:  bug_39257_v3.log added by avinoam (1KiB - application/octet-stream)
file #37889:  bug_39257_local_v3.cset added by lachlan (10KiB - text/x-diff)
file #36358:  bug_39257_local_v2.cset added by lachlan (10KiB - application/octet-stream)
file #36269:  bug_39256_local.cset added by lachlan (9KiB - 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 gyom (Posted a comment)
  • -email is unavailable- added by nicolasboumal (Posted a comment)
  • -email is unavailable- added by marsian (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by amro_octave
  • -email is unavailable- added by marsian
  • -email is unavailable- added by avinoam (Updated the item)
  • -email is unavailable- added by atcl (Posted a comment)
  • -email is unavailable- added by testplayer (Posted a comment)
  • -email is unavailable- added by qjuette (Posted a comment)
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 13 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-03-07 abarth Carbon-CopyRemoved 81675 => -
    2019-03-03 rik5 StatusReady For Test => Fixed
        Open/ClosedOpen => Closed
    2018-04-03 jwe StatusConfirmed => Ready For Test
    2018-03-29 avinoam Severity3 - Normal => 4 - Important
    2018-03-07 rik5 Carbon-Copy- => Added jwe
    2017-12-07 amro_octave Carbon-Copy- => Added amro_octave
    2017-11-14 marsian Carbon-Copy- => Added marsian
    2017-06-17 avinoam Attached File- => Added bug_39257_v3.log, #40931
    2016-07-15 lachlan Attached File- => Added bug_39257_local_v3.cset, #37889
    2016-02-16 lachlan Attached File- => Added bug_39257_local_v2.cset, #36358
    2016-02-07 lachlan Attached File- => Added bug_39256_local.cset, #36269
    2013-06-24 rik5 StatusNone => Confirmed

    Back to the top


    Powered by Savane 3.5