bugGNU Octave - Bugs: bug #51178, publish() sometimes removes last...

 
 

bug #51178: publish() sometimes removes last element from vector after section break

Submitter:  None
Submitted:  Sun 04 Jun 2017 01:06:29 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Tim Malte Gräfje Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 4.2.1
Operating System:  * GNU/Linux Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 07 Jun 2017 07:42:35 PM UTC, comment #7: 

I changed Octave's default precision to be 17 digits so that saving/restoring double values is now lossless by default.  See http://hg.savannah.gnu.org/hgweb/octave/rev/31b1ef1ee585.

Closing report.

Rik <rik5>
Group administrator
Tue 06 Jun 2017 08:07:37 PM UTC, comment #6: 

Perhaps the simplest way to fix this would be to change the default save precision from 16 to 17.  This is a configuration variable that can be altered with the save_precision() function call.

I verified with


save_precision (17);
tst_save_range


and it works correctly.

If this is the right way forward, then the line to change is ls-oct-text.cc:68


static int Vsave_precision = 16;



Rik <rik5>
Group administrator
Tue 06 Jun 2017 06:44:33 PM UTC, comment #5: 

Checking with the Wikipedia article on single precision (https://en.wikipedia.org/wiki/Single-precision_floating-point_format), Octave needs to store at least 9 digits of precision.  However, Octave has already set the precision to 16 for the stream so single values are being written with far more digits than necessary.  This isn't that much of a problem, the file size can grow a bit large, but one would use a binary format anyways if space were an issue.

Rik <rik5>
Group administrator
Tue 06 Jun 2017 06:15:49 PM UTC, comment #4: 

Checking with the wikipedia article on the IEEE-754 standard (https://en.wikipedia.org/wiki/Double-precision_floating-point_format), I find that precision is 15-17 decimal digits.  If Octave wants to guarantee lossless save/restore of values in text format it needs to be keeping 17 digits.  See the quote below.


This gives 15–17 significant decimal digits precision. If a decimal string with at most 15 significant digits is converted to IEEE 754 double precision representation and then converted back to a string with the same number of significant digits, then the final string should match the original. If an IEEE 754 double precision is converted to a decimal string with at least 17 significant digits and then converted back to double, then the final number must match the original.



Rik <rik5>
Group administrator
Tue 06 Jun 2017 04:43:22 PM UTC, comment #3: 

Octave should potentially be concerned that values are not be stored accurately.  I am not sure yet whether this is just a problem with ranges, or it is a general problem with all double values.

Currently, Octave uses the routine octave_write_double which internally relies on the C++ std::lib to format the output.  This routine writes 16 digits of precision, which would appear to be correct.


log10 (flintmax)
ans =  15.955


However, as a hack, I changed the routine to use 17 digits of precision.


diff -r 05bbca224a14 -r 36c3a3205e28 liboctave/util/lo-utils.cc
--- a/liboctave/util/lo-utils.cc        Tue Jun 06 08:50:20 2017 -0700
+++ b/liboctave/util/lo-utils.cc        Tue Jun 06 09:33:49 2017 -0700
@@ -381,6 +381,9 @@ template <> OCTAVE_API FloatComplex octa
   return octave_read_cx_fp_value<float> (is);
 }

+#include <stdio.h>
+#include "oct-locbuf.h"
+
 void
 octave_write_double (std::ostream& os, double d)
 {
@@ -391,7 +394,11 @@ octave_write_double (std::ostream& os, d
   else if (lo_ieee_isinf (d))
     os << (d < 0 ? "-Inf" : "Inf");
   else
-    os << d;
+    {
+      OCTAVE_LOCAL_BUFFER (char, tmp, 64);
+      sprintf (tmp, "%.17g", d);
+      os << tmp;
+    }
 }

 void


This fixes the issue, tst_save_range now returns 142 three times.  I've attached the diff for reference.

So the remaining question is whether Octave should enhance octave_write_double to be more precise, whether only the save_ascii routine for range variables should be updated, or whether this is really a problem on the load side.



(file #40864)

Rik <rik5>
Group administrator
Tue 06 Jun 2017 04:34:43 PM UTC, comment #2: 

The root cause of this bug is round-off error in a series of calculations.  The way the publish.m script works is to execute a block of code, checkpoint the current workspace by saving all the variables to a temporary file, format the resulting output for presentation, and then move to the next block of code.  For the next block of code it restores the current workspace by loading the variables from the temporary file.

Before my patch, publish.m was just using save/load to restore context.  Because there were no options, the file format used was that set by the 'save_default_options" command.  The default for this is '-text' which uses Octave's text format.  Unfortunately, it appears that this format does not always accurately store range variables.  To see this, I created the file tst_save_range.m which is attached.  The code is


tm = 29.43357091;
ta = 31 + (8/24);
tb = 31 + ((19+(45/60))/24);
dt = (5 / 60) / 24;
ts = ta:dt:tb;

numel(ts)

Mm = 1;
nm = 2;

save -text tmp.var
clear all
load tmp.var

############################################################
Ms = Mm + 2 * pi * nm * (ts - tm);
numel(Ms)

save -text tmp2.var
clear all
load tmp2.var

############################################################
numel (Ms)


The output is


ans =  142
ans =  142
ans =  141


By using a different format, such as '-binary' or '-v7', the code will work.  My immediate fix for publish.m was to switch to always using '-binary' format for saving intermediate variables.


Rik <rik5>
Group administrator
Tue 06 Jun 2017 03:59:14 PM UTC, comment #1: 

Confirmed.  This turns out to be a very subtle bug.

I've pushed a solution for publish on the stable branch here (http://hg.savannah.gnu.org/hgweb/octave/rev/8133a5041662).  This will be a part of the 4.2.2 bug fix release if there is one, or it will be a part of the next major release 4.4.0.  Since the change was only to an m-file you can download the file and replace your local copy if you need an immediate fix.

I'm adding jwe to the CC list for a continued discussion about the root cause of this bug.

Rik <rik5>
Group administrator
Sun 04 Jun 2017 01:06:29 PM UTC, original submission:  

Publishing the following code with publish('test.m') produces incorrect results.


tm = 29.43357091;
ta = 31 + (8/24);
tb = 31 + ((19+(45/60))/24);
dt = (5 / 60) / 24;
ts = ta:dt:tb;

numel(ts)

Mm = 1;
nm = 2;

%%
Ms = Mm + 2 * pi * nm * (ts - tm);
numel(Ms)

%%
numel(Ms)


Expected result:
The output of the numel() calls should be the same each time (142). Running the code from the octave command line produces this result but using the publish() function does not.

Actual result:
The last section break removes the last element from the Ms vector which causes the last numel() call to return 141 instead of 142.

Further information:
I am running Arch Linux.

The bug does not get triggered if you change the first three lines to


tm = 29.43357091 - 29;
ta = 31 - 29 + (8/24);
tb = 31 - 29 + ((19+(45/60))/24);


Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #40864:  write_dbl.diff added by rik5 (915B - 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 rik5 (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by None (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-06-07 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2017-06-06 rik5 Attached File- Added write_dbl.diff, #40864
    2017-06-06 rik5 StatusNone Confirmed
        Carbon-Copy- Added jwe

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code