bugGNU Octave - Bugs: bug #48243, Fix statistics computation in...

 
 

bug #48243: Fix statistics computation in ode45 and ode23

Submitter:  Jacopo Corno <jcorno>
Submitted:  Thu 16 Jun 2016 12:40:56 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Inaccurate Result
Status:  Fixed Assigned to:  None
Originator Name:  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

Tue 21 Jun 2016 03:19:11 PM UTC, comment #6: 

Okay, if it's been discussed and a resolution reached then we should just go implement it.  Besides, having fewer files to maintain is easier.

Rik <rik5>
Group administrator
Tue 21 Jun 2016 02:49:01 PM UTC, comment #5: 

Rik,

The removal of integrate_n_steps.m and integrate_const.m
was discussed already in the maintainers' list but I cannot
find the link right now.

Essentially these two serve no useful purpose, the only
useful extension to matlab's behaviour is to have the possibility
to specify at input the list of timesteps to use for integration, so it was decided to eliminate integrate_n_steps.m and integrate_const.m and maybe add integrate_fixed.m instead.

c.

Carlo de Falco <cdf>
Group Member
Tue 21 Jun 2016 01:52:29 PM UTC, comment #4: 

Octave aims to be a superset of Matlab, so if there is an interesting mathematical reason to have different underlying solvers, that would be okay as long as we preserve Matlab compatibility by having the default integrate_adapative option.

As for the bug fix number in the commit field, some people file a bug report just to get a number, and then use that immediately in the commit message.

Rik <rik5>
Group administrator
Tue 21 Jun 2016 09:22:13 AM UTC, comment #3: 

I confirm that integrate_n_steps.m and integrate_const.m are
to be removed sooner or later.
Of course as long as they exist it is not nice to produce incorrect statistics, maybe this is a good incentive to start working to remove them?

Carlo de Falco <cdf>
Group Member
Mon 20 Jun 2016 04:59:46 PM UTC, comment #2: 

Thanks Rik. I didn't spot the error.

Regarding the bug number I just spotted the bug and fixed it on the fly. For the future, should I create two separate istances? One for the bug and one for the fix?

I did not modify integrate_n_steps.m and integrate_const.m because I was under the assumption that it was decided to remove those two integrators since they are non Matlab-compatible.

If Carlo agrees I could prepare a patch for that change.


Jacopo Corno <jcorno>
Mon 20 Jun 2016 04:04:59 PM UTC, comment #1: 

@Jacopo: I modified your patch for ode statistics slightly and pushed it here (http://hg.savannah.gnu.org/hgweb/octave/rev/0f3e875d9078).

There was a semicolon missing which led to one of the statistics variable being printed.  My habit is to apply a patch and then run the BIST tests with, for example, 'test ode23' to make sure nothing was broken.  That's how I caught the semicolon.

Also, for the commit message, we like to reference the bug number in the commit message.  This makes it very easy to find which patch was used to fix a particular bug if things go wrong.


hg log -k 'bug #XXXXX'


One last thing, you modified private/integrate_adaptive.m.  There are two other functions, integrate_n_steps.m and integrate_const.m, in that directory.  Do they require fixing as well?

Rik <rik5>
Group administrator
Thu 16 Jun 2016 12:40:56 PM UTC, original submission:  

Fix statistics about number of steps, number of failed attempts and number of function evaluations in ode23 and ode45 solvers

Jacopo Corno <jcorno>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #37496:  odestat_cset.diff added by jcorno (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 cdf (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by jcorno
  • -email is unavailable- added by jcorno
  •  

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

    Date Changed by Updated Field Previous Value => Replaced by
    2016-06-20 rik5 StatusNone Fixed
        Open/ClosedOpen Closed
    2016-06-16 jcorno Attached File- Added odestat_cset.diff, #37496
        Carbon-Copy- Added -email is unavailable-
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code