test @sym/function_handle failed

[finalsnd]

Wrote file C:\Users\SnD\AppData\Local\Temp\oct_r1ACyg.m.
***** test
 % output to disk
 fprintf('\n')
 if (exist ('octave_config_info', 'builtin'))
   temp_file = tempname('', 'oct_');
 else
   temp_file = tempname();
 end
 % allow loading function from temp_file
 [temp_path, ans, ans] = fileparts(temp_file);
 addpath(temp_path);
 f = function_handle(2*x*y, 2^x, 'vars', {x y z}, 'file', temp_file);
 assert( isa(f, 'function_handle'))
 [a,b] = f(10,20,30);
 assert (isnumeric (a) && isnumeric (b))
 assert (a == 400)
 assert (b == 1024)
 assert (unlink([temp_file '.m']) == 0)
 % remove temp_path from load path
 rmpath(temp_path);
!!!!! test failed
@oct_r1ACyg: no function and no method found
shared variables
  scalar structure containing the fields:

x = (sym) x
y = (sym) y
z = (sym) z

[cbm755]

@genuinelucifer any idea what's going on here? of those three tests, one or two of them seem to fail.

@finalsnd would you be willing to step through those tests manually, look at the path, see if you can narrow it down a bit?

[genuinelucifer]

It seems like a file reading problem.
.
if I run the one of the tests that creates a file then I get error...
But if I call function_handle again, that is, duplicate the call in the test then it works!!

>> syms x y z
>> if (exist ('octave_config_info', 'builtin'))

  temp_file = tempname('', 'oct_');

else

  temp_file = tempname();

end

>> % allow loading function from temp_file

>> [temp_path, ans, ans] = fileparts(temp_file);

>> addpath(temp_path);

>> f = function_handle(2*x*y, 2^x, 'vars', {x y z}, 'file', temp_file);

error: @oct_QRUkvQ: no function and no method found
error: called from
    function_handle at line 134 column 7
>> f = function_handle(2*x*y, 2^x, 'vars', {x y z}, 'file', temp_file);

>> f
f = @oct_QRUkvQ

[genuinelucifer]

@finalsnd You can use #432 till we figure out a better solution. Now, it takes more time though (approx 40 sec for test @sym/function_handle.m to run on my machine)...

[cbm755]

Weird. Should we be checking the return value of fclose?

[genuinelucifer]

Weird. Should we be checking the return value of fclose?

Probably... But, seeing the return value, still would require to do a check on the file. Or, is there an overload which flushes writing the file if there's a return value?

[finalsnd]

Maybe Octave doesn't refresh the files in the path until a few seconds.

There's a function, ignore_function_time_stamp() but i'm changing it and doesn't work anyway.

Built-in Function: val = ignore_function_time_stamp ()
Built-in Function: old_val = ignore_function_time_stamp (new_val)
Query or set the internal variable that controls whether Octave checks the time stamp on files each time it looks up functions defined in function files.

If the internal variable is set to "system", Octave will not automatically recompile function files in subdirectories of octave-home/lib/version if they have changed since they were last compiled, but will recompile other function files in the search path if they change.

If set to "all", Octave will not recompile any function files unless their definitions are removed with clear.

If set to "none", Octave will always check time stamps on files to determine whether functions defined in function files need to recompiled.

[cbm755]

What is the fclose return value? Should have 'assert ( ret == 0)' or similar

Does it help if we write the file then add the path?

[genuinelucifer]

What is the fclose return value?

It is zero only.

Does it help if we write the file then add the path?

Indeed! It works...
But then we will have to addpath in function_handle and not in tests! Will it be fine?

[genuinelucifer]

Maybe Octave doesn't refresh the files in the path until a few seconds.

@finalsnd Yes it seems so. @mtmiller also suggested a similar thing related to timestamp of the directory not being changed by windows...

[genuinelucifer]

@cbm755 If we add the path in function_handle, then we can also return the path to the user (preferably as optional return value) so that user could remove it from path if required.

[finalsnd]

There's no need for that. The user already knows the path of the file, doesnt?

Maybe it's good idea to remove the path after finish the function, or leave it alone if the path existed before invoking the function.

[genuinelucifer]

There's no need for that. The user
already knows the path of the file,
doesnt?

Fair enough

Maybe it's good idea to remove the
path after finish the function, or leave it
alone if the path existed before
invoking the function.

I'll have to try that. But, if we remove the path then the function handle
won't work, will it? Probably leaving it in path would be the better
solution at this point?

[finalsnd]

I think that if the user didn't set the path before invoking the function, then it will be an error (octave can't find, for example, temp_file).

So, not need to remove the path anyway. Or am i missing something?

Is there any way to invoke a function that isn't in the path? (something like 'run' but for functions)

[mtmiller]

IMHO the test as written is correct, and works on GNU/Linux and probably any POSIX, it may be something we don't quite understand about the Windows filesystem or I/O layer that is not behaving the way Octave expects.

@finalsnd Not now, but in a future version of Octave the run function should allow a function in an arbitrary directory to be executed. See https://savannah.gnu.org/bugs/?47807. Not sure how that helps in this case, though.

[cbm755]

But then we will have to addpath in function_handle and not in tests! Will it be fine?

@genuinelucifer Umm, no that's not fine. I just meant:

%! h = function_handle(H, M, V, 'vars', {x y z}, 'file', temp_file);
%! pause(0.5)
%! addpath(temp_path);

@sym/function_handle should not be messing with anyone's path.

[genuinelucifer]

No actually, that wouldn't work. We call `str2func` in function_handle and hence the path should be modified before that call.... I guess we will have to look into how octave loads the files and probably add a new method to reload the cache manually.

[cbm755]

I see. What about writing the test to chdir to temp_data before calling function_handle?

[genuinelucifer]

What about writing the test to chdir to temp_data before calling function_handle?

Then the call to function_handle fails if you do not have the package installed.. I run octave in inst dir and it fails for me.
And after that, I installed symbolic from forge and it still fails if I run the code of the test (with the edit):

>> cd(temp_path);

>> f = function_handle(2*x*y, 2^x, 'vars', {x y z}, 'file', temp_file);

Wrote file C:\Users\ABHINA~1\AppData\Local\Temp\oct_PuDlZI.m.
error: @C:\Users\ABHINA~1\AppData\Local\Temp\oct_PuDlZI: no function and no method found
error: called from
    function_handle at line 142 column 7

[mtmiller]

The directory or the path doesn't seem to be the issue, I think the core of this issue is that the something ilke following may fail on Windows (but one of you please verify for me).

fid = fopen ("foo.m", "wt");
fputs (fid, "function foo ()\n  disp (\"This is function foo.\");\nendfunction\n");
fclose (fid);
fn = str2func ("foo");
fn ()

This is expected to work in Octave, at the command line, in one long command, as a script, in any context. If it doesn't, I think we should address this underlying bug instead of (or before) trying to invent workarounds.

[genuinelucifer]

fid = fopen ("foo.m", "wt");
fputs (fid, "function foo ()\n disp ("This is function
foo.");\nendfunction\n");
fclose (fid);
fn = str2func ("foo");
fn ()

This is expected to work in Octave

It actually works!!!
Although if in place of the first line I write:

addpath(" e:\\");
fid = fopen("e:\\foo.m");

then it has the same problem. I have to use a delay before str2func can be
called.

Seems like octave loads the current directory asap but it doesn't load the
cache of other directories in path untill some time.

[finalsnd]

The problem is that Octave in Windows doesn't refresh instantly the new functions in the files of the path. If you wait a while, you can run the new function and works fine.

@genuinelucifer tried to put the addpath after creating the file and it worked. Maybe because when you add a path, octave refreshes all the functions.

That's why i said maybe changing ignore_function_time_stamp() would fix it, but i tried and didn't fix anything.

@mtmiller the function 'run' doesn't accept input parameters... so we can't use it for this.

That code didn't fail for me!!!

[genuinelucifer]

I tried a few more times and can now confirm that addpath then fopen fails. But fclose then addpath works! (for files in different directory than current) Seems like addpatb refreshes the cache. We might just need to define a new function that would refresh the cache for us. Or maybe when functions like str2func query for functions and variables in path, a refresh should occur automatically?!

[finalsnd]

That's what rehash supposed to do... and the ignore_function_time_stamp... doesn't?

I opened a bug report in Octave (I forgot to fill the title... :( ):

https://savannah.gnu.org/bugs/index.php?47818

[mtmiller]

The function rehash is supposed to go through the directories in the path and check them for new functions, but still only if the timestamp of the directory is newer than the last time it was checked. The variable ignore_function_time_stamp only applies to files that have already been parsed and are stored in the symbol table, not to completely new functions that appear in a directory in path.

Octave still fundamentally depends on the timestamp of the directory containing an m-file to be updated when a new file is created or else it will never look in the contents of the directory for any functions that it hasn't already seen.

[mtmiller]

Anyway, we should continue this at the appropriate bug report.

[genuinelucifer]

@mtmiller Could you please point me to some file where I could find the implementation of addpath and rehash? I cloned octave from mercurial and went through many of the files but couldn't find anything related... I am looking into octave code for the first time and have no clue how the files are put together. I'll try though, but it would really help if there would be some documentation about how to approach the files...

[mtmiller]

@genuinelucifer Sure, let's discuss in the appropriate venue, either bug #31080 or the maintainers list, your choice.

[genuinelucifer]

I sent the mail on maintainers list. That would be more suitable for me....

[genuinelucifer]

Well, I have been looking at the octave source. The only solution that comes to my mind is have a force_rehash() or better force_rehash(dir_name) or even better rehash(true, dir_name) which wouldn't look for time stamp to update?
It will involve just copying around the function load_path::dir_info::update (void) in loadpath.cc
.
@cbm755 @mtmiller Should I do that? Or do we need some other solution?
Also, if I do that, how do I send PR to octave? I am familiar with git, but have no clue about mercurial. (I'm looking into some mercurial tutorials right now, so I might figure it out in due time).

[genuinelucifer]

And even if we do that, it will probably take a long time before it actually gets officially into octave release (i guess so as is the case with most of the large open source softwares)...
Till then, we would need some temporary solution....? What should we do about that?

[cbm755]

I haven't been following this enough to know if that is the correct solution upstream or not. But probably should discuss at the upstream bug report.

Re: tests: I'm not too worried about the failing tests on Windows. I guess in real-life, people will use function_handle interactively where its less of a problem... We could just mark them xtest and leave a comment with a url for the upstream bug: %! % these sometimes fail on Windows: https://savanahh...

[genuinelucifer]

We could just mark them xtest

Certainly seems like the most favourable option at this point. I have posted on the bug report and will see what the response of other people is.
.
Just asking, can't we check for the OS in the test. If it is windows then use #432 else leave it? (i mean working test, although slow, would still be better, right?)

[mtmiller]

I agree that even if this important issue is fixed upstream, it will be worth commenting or addressing in some way in octsympy, because the fix likely won't help a lot of users for a while.

The 3 %!tests for function_handle(…, 'file', …) can just have a big if (~ispc) around them, if you want to avoid having failing tests, and a FIXME comment to revisit when this problem is fixed later.

[cbm755]

Alright, I'd be ok with #432 wrapped with ispc() and the if(exist('OCTAVE_VERSION')...). And a comment pointing to the upstream report.

[genuinelucifer]

@mtmiller Sorry, at this point I do not wish to dive into windows file systems and researching into the filestamps (as you suggested on the bug report). Maybe sometime down the line or after GSoC, I could try this.
.
@cbm755 could you please explain if(exist('OCTAVE_VERSION')...) ? why to check existence of octave_version?

[cbm755]

could you please explain |if(exist('OCTAVE_VERSION')...)| ? why to check existence of octave_version?

Because Octsympy supports Matlab too. This is the same reason we don't use Octave-only extensions like `endfunction` and double-quoted strings.