bugGNU Octave - Bugs: bug #56941, format rat or rats display 0...

 
 

bug #56941: format rat or rats display 0 rather than rational approximation for small numbers

Submitter:  Rik <rik5>
Submitted:  Mon 23 Sep 2019 08:01:33 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect 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

Sun 06 Oct 2019 01:52:29 AM UTC, comment #9: 

Yes, it did.  I've had a side conversation with jwe about this.  For the moment, I'll just mark it as a known bug (https://hg.savannah.gnu.org/hgweb/octave/rev/f429338b9f85), but I do have a few ways co fix this.

Rik <rik5>
Group administrator
Sat 05 Oct 2019 01:09:18 AM UTC, comment #8: 

Did it just break

>>>>> processing /home/buildbotu/fc25-x86_64/gcc-lto-fedora/build/libinterp/corefcn/pr-output.cc-tst

*** test
 [old_fmt, old_spacing] = format ();
 unwind_protect
   format short;
   assert (rats (2.0005, 9), "4001/2000");
   assert (rats (-2.0005, 10), "-4001/2000");
   assert (strtrim (rats (2.0005, 30)), "4001/2000");
   assert (pi - str2num (rats (pi, 30)), 0, 4 * eps);
   assert (e - str2num (rats (e, 30)), 0, 4 * eps);
   assert (rats (123, 2), " *");
   v = 1 / double (intmax);
   err = v - str2num (rats (v, 12));
   assert (err, 0, 4 * eps);
 unwind_protect_cleanup
   format (old_fmt);
   format (old_spacing);
 end_unwind_protect
!!!!! test failed
ASSERT errors for:  assert (rats (2.0005, 9),"4001/2000")
  Location  |  Observed  |  Expected  |  Reason
     []               2    4001/2000     Strings don't match

Dmitri A. Sergatskov <dasergatskov>
Fri 04 Oct 2019 09:12:08 PM UTC, comment #7: 

This changeset (https://hg.savannah.gnu.org/hgweb/octave/rev/d503426130bf) resolves the issue specified (displaying '0' rather than '*' for small rational approximations) and mostly makes Octave Matlab-compatible.  Closing report.

Rik <rik5>
Group administrator
Fri 04 Oct 2019 06:09:15 PM UTC, comment #6: 

This is going to be rather difficult to fully fix short of overhauling all of the rational print routines.

In Matlab, they call the equivalent of rat with a specific tolerance


[N,D] = rat(X,tol), where tol is min(10^(-(strlen-1)/2)*norm(X(isfinite(X)),1),.1)


Next they form the string "N/D" and then if that generated string is greater than strlen they display "*".

In pr-output.cc, Octave calls out to the function rational_approx (val, strlen) which is in liboctave/util/oct-string.cc.  This function generates a string of no more than the specified length (strlen).  This produces strange results because a small positive number which can just be represented in the desired length will be returned, but a small negative number with the same representation, is returned as '0' because the minus sign makes it one longer than the requested strlen.

Additionaly, the calling routine in pr-output.cc then looks to see if the returned string is greater than strlen, and if so replaces it with '*'.  But this doesn't happen for small numbers because the earlier routine has already clipped the result and exited when this condition is met.

I can make a few alterations to improve things, but full Matlab-compatibility would require a big rewrite.

Rik <rik5>
Group administrator
Tue 24 Sep 2019 03:33:11 AM UTC, comment #5: 

It seems that rat.m and the C++ code in oct-string.cc do not produce the same result.  This is a problem.  In terms of engineering principles, we really should only have one code path for this and then re-use it.  In any case, rat.m seems to get the same answer as Matlab.


[n,d] = rat ([-pi, 0, pi] * 1e-6)
n =

         -1          0          1

d =

     318310          1     318310



Rik <rik5>
Group administrator
Mon 23 Sep 2019 11:50:52 PM UTC, comment #4: 

Fine to separate rat/rats if desired.  I assume that Matlab's "format rat" uses rat() or rats() internally which is why I put them together.

Cusp of switchover in Matlab is 1e-6.  This produces asterisks but 1e-5 does not.


y = [1e-6, 1e-6]
y = [       *         *       ];



Rik <rik5>
Group administrator
Mon 23 Sep 2019 09:51:25 PM UTC, comment #3: 

It's pretty easy to show by example that Matlab has different rules for 'format rat' output vs the 'rats' function.

In Matlab (2019b):


>> x = pi * 1e-9;
>> format rat;
>> x

x =

       1/318309886

>> rats(x)

ans = '       *      '


It may be helpful to discuss 'format rat' and 'rats' separately, or at least be careful in distinguishing the behavior of each.

Mike Miller <mtmiller>
Group Member
Mon 23 Sep 2019 09:25:29 PM UTC, comment #2: 

Matlab also seems to have an output limit somewhere. Matlab returns '1' for 'format rat, 1 - 1e-7', but returns a fraction for '1 - 1e-6'.

Mike Miller <mtmiller>
Group Member
Mon 23 Sep 2019 08:38:14 PM UTC, comment #1: 

I'm not sure how to print the contents of an ostringstream object with gdb, so I used the simple change shown below.  I found that Octave computes the same value as Matlab for the test case but then bails out and returns "0" because the output is considered too wide.  I don't know what the proper fix is since I'm not sure why we impose a limit on the output width.


diff --git a/liboctave/util/oct-string.cc b/liboctave/util/oct-string.cc
--- a/liboctave/util/oct-string.cc
+++ b/liboctave/util/oct-string.cc
@@ -23,6 +23,8 @@ along with Octave; see the file COPYING.
 #  include "config.h"
 #endif

+#include <iostream>
+
 #include "oct-string.h"

 #include <algorithm>
@@ -624,8 +626,12 @@ rational_approx (T val, int len)
               if (buf.str ().length () > static_cast<unsigned int> (len + 2))
                 break;
             }
-          else if (buf.str ().length () > static_cast<unsigned int> (len))
-            break;
+          else
+            {
+              std::cerr << "buf: " << buf.str () << std::endl;
+              if (buf.str ().length () > static_cast<unsigned int> (len))
+                break;
+            }

           if (std::abs (n) > std::numeric_limits<int>::max ()
               || std::abs (d) > std::numeric_limits<int>::max ())


John W. Eaton <jwe>
Group administrator
Mon 23 Sep 2019 08:01:33 PM UTC, original submission:  

For small numbers, the 'rat' format prints '0' rather than either a rational approximation or simply an indication that it cannot display the correct value.

As an example,


octave:43> format rat
octave:44> x = pi*1e-9
x = 0


However, Matlab displays


1/318309886


which indeed is a reasonable approximation.

Rik <rik5>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (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 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-10-04 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code