bugGNU Octave - Bugs: bug #59477, __ode15__.cc does not consider the...

 
 

bug #59477: __ode15__.cc does not consider the number of arguments in the event function

Submitted by:  Hg200 <hg200>
Submitted on:  Tue 17 Nov 2020 11:15:37 PM UTC  
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  None Assigned to:  None
Originator Name:  Hg200 Open/Closed:  Open
Release:  dev Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

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

 

Wed 18 Nov 2020 10:23:19 PM UTC, comment #3: 

Thank you very much for the feedback. Your solution works very well. I like it because it is less code-invasive than making the change in the C code. The question about the runtime is a bit difficult to answer because it depends on how often the event function is called (i.e. how many steps have to be calculated) compared to the initialization overhead. To get some numbers I made a tic / toc measurement over five consecutive program calls. The used programs are attached. I have also attached the changes I made in __ode15__.cc for the measurement. This should be what you suggest in comment #2.

ode15s()
--------
i.) 2 to 3 wrapper in ode15s.m (odf15s-events-diff.txt)
6.31828+6.35755+6.25858+6.40452+6.29574

ii.) Adjust number of event function parameters in __ode15__.cc (events-c-solution-diff.txt)
5.80774+5.84905+5.73096+5.90781+6.24962

Conclusion: Wrapper 7% slower than C solution.

ode15i()
--------
i.) 2 to 3 wrapper in ode15s.m (has no effect on ode15i)
5.63038+5.9534+5.66035+5.75075+5.6973

ii.) Adjust number of event function parameters in __ode15__.cc (events-c-solution-diff.txt)
5.53875+5.62682+5.56293+5.62556+5.66299

Conclusion: Wrapper 2% slower than C solution (wrapper should not add overhead in ode15i).

In my opinion both solutions are ok. Assuming that the measurement is ok, I tend a little bit more to the simplicity at the expense of the runtime.

(file #50297, file #50298, file #50299)

Hg200 <hg200>
Wed 18 Nov 2020 03:11:14 PM UTC, comment #2: 

The attached patch should work, but at the cost of an additional function handle evaluation for each call to the events function.  If that is too much, then I think the better solution would be to change _ode15_ to call the events function with the correct number of inputs depending on whether it is called for ode15i or ode15s.  If we go that direction, I would do it by adding a parameter or option to _ode15_, not by examining the call stack.

(file #50296)

John W. Eaton <jwe>
Project Administrator
Wed 18 Nov 2020 03:02:57 AM UTC, comment #1: 

If the event function can be a handle that refers to a MEX or .oct file function, then I don't think we can always have a way to ask it how many arguments it can accept.  So it seems _ode15_ needs to know whether it is being called to do the work of ode15i or ode15s so it can make the proper call, or ode15s needs to do something like

options.Events = @(t, y, ~) options.Events (t, y);

to place a wrapper that can accept 3 arguments but only passes 2 of them to the event function provided by the user.

John W. Eaton <jwe>
Project Administrator
Tue 17 Nov 2020 11:15:37 PM UTC, original submission:  

This is an interface problem in the function ode15s(), which manifests since cset 28562:b0b80efecea1 "error for functions called with too many inputs or outputs". The attached minimal example for ode15s() runs fine with Matlab R2018 and also with Octave 6.0.9x, but it fails on the development branch. The reason for the error is the following:

According to Matlab and its documentation the event function of ode15s() has two input parameters:

ode_events (t, y)

But the event function of ode15i() has three input parameters:

ode_events (t, y, yp)

ode15s() and ode15i() are both implemented in the file __ode15__.cc. In __ode15__.cc around line 766 (IDA::event), however, the event function is evaluated with three input parameters:

octave_value_list args = ovl (tsol, y, yp);
...
octave_value_list output = feval (event_fcn, args, 3);

In the case of ode15s(), however, the event callback has only two parameters. Therefore you get following error when running the attached example on default branch:

error: ode_events: function called with too many inputs
error: called from
    ode15sfun>ode_events
    ode15s at line 315 column 22
    ode15sfun at line 14 column 22

The relation to cset b0b80efecea1 is, that before b0b80efecea1 the number of arguments was not checked when calling a function. But now they are checked and if they do not match, the error is thrown. Furthermore b0b80efecea1 is not merged to stable and therefore this problem does not yet manifest on stable.

Fixing ideas: If we want to maintain the use of "yp" in ode15s() we need to determine the number of input arguments of "event_fcn" and based on this we could compute different sized "args" in IDA::event. In pseudo code something like this:

if (nargin(@event_fcn) == 2)
  octave_value_list args = ovl (tsol, y);
else
  octave_value_list args = ovl (tsol, y, yp);
...
octave_value_list output = feval (event_fcn, args, 3);

Typically in Octave C code the number of input arguments "nargin" of a function is taken from the argument list "args.length ()". But we only have the function handle. Is it possible to determine the number of input arguments also from a pure function handle?

Other ideas to fix this issue?

Hg200 <hg200>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #50298:  ode15i_runtime.m added by hg200 (1KiB - text/x-objcsrc)
file #50299:  ode15s_runtime.m added by hg200 (829B - text/x-objcsrc)
file #50296:  odf15s-events-diff.txt added by jwe (2KiB - text/plain)
file #50293:  ode15sfun.m added by hg200 (612B - text/x-objcsrc)

 

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 hg200 (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-11-18 hg200 Attached File- => Added events-c-solution-diff.txt, #50297
        Attached File- => Added ode15i_runtime.m, #50298
        Attached File- => Added ode15s_runtime.m, #50299
    2020-11-18 jwe Attached File- => Added odf15s-events-diff.txt, #50296
    2020-11-17 hg200 Attached File- => Added ode15sfun.m, #50293

    Back to the top


    Powered by Savane 3.5