Re: Moving ode changes to default branch

[c.]

On 11 Oct 2016, at 17:55, Rik <address@hidden> wrote:

> 10/11/16
> 
> Carlo,
> 
> I'm concerned that the latest changes to the ode functions made just five
> days ago are too speculative for the stable branch and should be re-located
> to the development branch.  As Carnë noted in a separate e-mail, feature
> freeze has already happened and the stable branch is now just for bug fixes
> and documentation updates.
> 
> I'm not saying that the changes are bad, but it's clear that there hasn't
> been enough time to test things.  The first commit broke 'make check' and
> that had to be quickly patched.  If I look at the code for odedefaults.m I
> see that there is no indentation for code within the function block, single
> quotes are used instead of double quotes, and there is no space between the
> function name and the opening parenthesis.  This is small stuff, and can be
> corrected, but it is an indication that a thorough review has not
> happened.  Similarly, ode23.m has a typo in the docstring:
> <@qcode{"AbsTol"},.>.  The comma before the period is not grammatically
> correct.  With more testing time I'm sure this would be caught.  Finally, I
> did some benchmarking of odeset because I remember that I had to do some
> tricks to get it to run quickly.  The following code
> 
> tic; x = odeset (); toc
> 
> runs in 0.00015 seconds with the old code, but now takes .0058 seconds. 
> That's a 39X slowdown.  Of course the absolute time of five milliseconds is
> probably unimportant to a human, but I want to understand whether we must
> accept that slowdown, or whether there is a better way to code this. 
> Perhaps the use of persistent variables in odedefaults would help.
> 
> If you have a plan on how we could become comfortable with the idea of
> keeping this code on the stable branch we will listen.  Otherwise, it seems
> like the best course is to move the changes to the default branch where
> they can simply have more time to mature into solid code.
> 
> --Rik

Rik,

Sorry I couldn't answer before but it has been a really crazy week.

I hope the changes have not been reverted yet and I still have time
to try and convince you that these changes should be in the release.

I'll provide more details later but, to make a long story short,
the main point is that this version of the functions is actually
more "stable" than the previous one, it has undergone more testing
and contains fixes to known bugs.

As for the poor speed performance of odeset:

- odeset is not meant to be a performance critical function, in a 
  standard use scenario the time taken by odeset is a negligible
  fraction of the simulation time

- the performance hit is that large only the first time odeset is run,
  after the first invocation it runs about twice as fast on my system

- more importantly, the behaviour of the previous implementation was
  not what we wanted, it did cause some tests to fail but they went 
  unnoticed as they were marked xfail.

- finally odepkg functions depend on odeset behaving correctly, if this
  version of odeset is not accepted I am afraid neither the current development
  version nor the previous release of odepkg will work with 4.2 so we won't
  have a working odepkg until next release.

c.