bugGNU Octave - Bugs: bug #57351, str2func no longer emits error on...

 
 

bug #57351: str2func no longer emits error on non-existent function input

Submitter:  John Donoghue <lostbard>
Submitted:  Tue 03 Dec 2019 09:22:48 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  JohnD Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 05 Feb 2020 01:05:51 PM UTC, comment #13: 

Those changes look good to me. The buildbots seem to be happy with them, too.
Closing as fixed.

Markus Mützel <mmuetzel>
Group administrator
Wed 05 Feb 2020 01:12:59 AM UTC, comment #12: 

I don't think keeping this bug report open as a reference for a list of Octave Forge packages is useful to anyone. Whenever you're satisfied that everything in Octave itself is working, we should close this as fixed. If someone's interested in the task of investigating str2func in all Forge packages, they can take my list as a starting point and go from there.

I expect any issues with str2func will be input validation type bugs that will mostly be found as Octave 6 rolls out.

Mike Miller <mtmiller>
Group Member
Wed 05 Feb 2020 12:31:35 AM UTC, comment #11: 

I think this additional changeset should stop the errors in 'make check': https://hg.savannah.gnu.org/hgweb/octave/rev/ace8dc642e4d

Rik <rik5>
Group administrator
Tue 04 Feb 2020 07:53:57 PM UTC, comment #10: 

Probably my doing.  I don't have SUNDIALS installed so my 'make check' didn't run the new BIST tests.  I'll take a look.

Rik <rik5>
Group administrator
Tue 04 Feb 2020 08:27:43 AM UTC, comment #9: 

Some of the BISTs for ode15s are failing. This might be related to the last change here.

>>>>> processing /scratch/buildbot/workers/jwe-debian-x86_64-2/gcc-7-debian/src/scripts/ode/ode15s.m
***** testif HAVE_SUNDIALS
 opt = odeset ("MStateDependence", "none",
               "Mass", [1, 0, 0; 0, 1, 0; 0, 0, 0]);
 [t, y] = ode15s (@rob, [0, 100], [1; 0; 0], opt);
 assert ([t(end), y(end,:)], frefrob, 1e-3);
!!!!! test failed
ode15s: invalid value assigned to field 'Mass'
***** testif HAVE_SUNDIALS
 opt = odeset ("MStateDependence", "none",
               "Mass", sparse ([1, 0, 0; 0, 1, 0; 0, 0, 0]));
 [t, y] = ode15s (@rob, [0, 100], [1; 0; 0], opt);
 assert ([t(end), y(end,:)], frefrob, 1e-3);
!!!!! test failed
ode15s: invalid value assigned to field 'Mass'
***** testif HAVE_SUNDIALS
 opt = odeset ("MStateDependence", "none",
               "Mass", [1, 0, 0; 0, 1, 0; 0, 0, 0],
               "Jacobian", "jacfundense");
 [t, y] = ode15s (@rob, [0, 100], [1; 0; 0], opt);
 assert ([t(end), y(end,:)], frefrob, 1e-3);
!!!!! test failed
ode15s: invalid value assigned to field 'Mass'
***** testif HAVE_SUNDIALS
 opt = odeset ("MStateDependence", "none",
               "Mass", sparse ([1, 0, 0; 0, 1, 0; 0, 0, 0]),
               "Jacobian", @jacfundense);
 [t, y] = ode15s (@rob, [0, 100], [1; 0; 0], opt);
 assert ([t(end), y(end,:)], frefrob, 1e-3);
!!!!! test failed
ode15s: invalid value assigned to field 'Mass'
***** testif HAVE_SUNDIALS
 opt = odeset ("Mass", eye (2,2), "MStateDependence", "none");
 sol = ode15s (@fpol, [0, 2], [2, 0], opt);
 assert ([sol.x(end), sol.y(end,:)], [2, fref], 3e-3);
!!!!! test failed
ode15s: invalid value assigned to field 'Mass'
***** testif HAVE_SUNDIALS
 opt = odeset ("Mass", speye (2), "MStateDependence", "none");
 sol = ode15s (@fpol, [0, 2], [2, 0], opt);
 assert ([sol.x(end), sol.y(end,:)], [2, fref], 3e-3);
!!!!! test failed
ode15s: invalid value assigned to field 'Mass'


Markus Mützel <mmuetzel>
Group administrator
Tue 04 Feb 2020 04:25:04 AM UTC, comment #8: 

I checked in fixes for the input validation in core Octave that depended on str2func().  See https://hg.savannah.gnu.org/hgweb/octave/rev/5e44268dca6f.  The remaining instances in Octave Forge packages I leave to the maintainers of the packages.

Rik <rik5>
Group administrator
Thu 30 Jan 2020 11:02:52 PM UTC, comment #7: 

A list of all Octave Forge packages that call str2func in any context:

  • arduino
  • benchmark
  • fuzzy-logic-toolkit
  • image
  • miscellaneous
  • odepkg
  • optim
  • parallel
  • statistics
  • stk
  • symbolic


Mike Miller <mtmiller>
Group Member
Thu 30 Jan 2020 10:59:30 PM UTC, comment #6: 

I just ran a quick grep over all Octave Forge packages, and the 'odepkg' package is the only one where I still see str2func used in a try-catch block for validation. But I also think the Octave Forge 'odepkg' is a stale copy and development has moved to an external repository, not sure where that is now.

But other packages do use str2func without try-catch, and with the error delayed until invocation now, the behavior may be different.

Mike Miller <mtmiller>
Group Member
Wed 29 Jan 2020 03:18:35 PM UTC, comment #5: 

It shouldn't be closed.  There is no issue with the str2func() function.  However, there are several functions which rely on the old behavior for input validation, which they should no longer do.

What is required is a review and possible tweak to the input validation for ode15i.m, ode15s.m, ode23.m, ode45.m, and ode/private/check_default_input.m.  There may be packages which are also incorrectly coded, but I only used grep with core Octave.

If one requires that the function be present when the odeXX.m file is called then exist() should be used instead of the try/catch block sequence.  If it is not important then the entire try/catch block can be eliminated and, for the code in comment #2, replaced with


options.Jacobian = str2func (options.Jacobian);


No one who wrote these routines is on the CC list for this bug report.  It might be a good idea to post to the Octave Maintainer's list so that Carlo de Falco, Marco Caliari, Francesco Faccio, or Jacopo Corno sees it.

Rik <rik5>
Group administrator
Wed 29 Jan 2020 10:43:25 AM UTC, comment #4: 

Is there anything else that needs to be done for this bug report?  Should it be closed as invalid since the change to Octave was intentional and needed for Matlab compatibility?

John W. Eaton <jwe>
Group administrator
Tue 03 Dec 2019 11:34:49 PM UTC, comment #3: 

John-- Your patch for statistics looks good, I applied it.

Nir Krakauer <nir_krakauer>
Tue 03 Dec 2019 10:22:24 PM UTC, comment #2: 

This change was made to support Matlab which does late-binding of name to function.  This is useful because you can declare a handle to a function, without having that function immediately available.  Only when you actually attempt to execute the function does the interpreter check for existence.

Using grep in the scripts/ directory I find that ode15i.m, ode15s.m, ode23.m, ode45.m, and ode/private/check_default_input.m likely have the same issue.  Here is an example


if (ischar (options.Jacobian))
  try
    options.Jacobian = str2func (options.Jacobian);
  catch
    warning (lasterr);
  end_try_catch
  if (! is_function_handle (options.Jacobian))
    error ("Octave:invalid-input-arg",
           [solver ": invalid value assigned to field 'Jacobian'"]);
  endif




Rik <rik5>
Group administrator
Tue 03 Dec 2019 10:04:16 PM UTC, comment #1: 

The statistics package at least was relying on
str2func throwing an error if attempting to assign to an invalid function name in order decide on what function name to use.

Patch attached for the statistics package that checks for the functions existance rather than relying on the error occuring.



(file #47985)

John Donoghue <lostbard>
Group Member
Tue 03 Dec 2019 09:22:48 PM UTC, original submission:  

From recent changes to dev octave, the str2func no longer throws an error when attempting to create a function handle to a non existing function.


a = str2func('unknown_func_name')


In Octave 5.1, this would cause on an error to an for no function or method found.


In octave 6, it just returns the string @unknown_func_name

John Donoghue <lostbard>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #47985:  stats.patch added by lostbard (1KiB - 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 mmuetzel (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by nir_krakauer (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by lostbard (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-02-05 mmuetzel StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2020-02-05 mtmiller CategoryOctave Package Octave Function
        StatusConfirmed Ready For Test
    2020-02-04 rik5 CategoryOctave Function Octave Package
    2019-12-03 rik5 Item GroupNone Other
        StatusNone Confirmed
        Summarystr2func behavour changes in dev octave str2func no longer emits error on non-existent function input
    2019-12-03 lostbard Attached File- Added stats.patch, #47985

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code