bugGNU Octave - Bugs: bug #55682, round(X,N) and round(X,N,type)

 
 

bug #55682: round(X,N) and round(X,N,type)

Submitted by:  Joshua Gay <josh>
Submitted on:  Sat 09 Feb 2019 01:30:37 PM UTC  
 
Category:  Octave Function Severity:  1 - Wish
Priority:  3 - Low Item Group:  Feature Request
Status:  Confirmed Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  dev Operating System:  GNU/Linux

Add a New Comment (Rich Markup)
   

You are not logged in

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

 

( Jump to the original submission)

Sat 25 May 2019 11:59:41 PM UTC, comment #10: 

Sigh

It is very clean that way, but I had hoped to limit the re-writing of the API.  This will require propagating a two-input round form up through libinterp and back down in to liboctave.  Presumably we should also maintain a legacy zero-input form, or arrange for defaults in the function prototype for the 2-input form such that it can be called with no arguments.

Can't use ceil() in place of round because it always rounds in the direction of positive infinity.

ceil (-1.5) == -1
round (-1.5) == -2

Rik <rik5>
Project Administrator
Sat 25 May 2019 12:07:01 PM UTC, comment #9: 

I would try to limit the code in the Fround function to just doing argument parsing.  Something like:

{
  int nargin = args.length ();

  if (nargin < 1 || nargin > 3)
    print_usage ();

  if (nargin == 1)
    return ovl (args(0).ceil ());

  int n = args(1).xint_value ("N must be an integer value");

  enum octave::rounding_mode = octave::round_normal;
  if (nargin == 3)
    {
      std::string mode = xstring_value ("MODE must be a string");

      // Case insensitive compare?  Abbreviations?
      if (mode == "significant")
        mode = octave::round_significant;
      else if (mode == "decimals")
        mode = octave::round_decimals;
      else
        error ("round: MODE must be either \"decimals\" or \"significant\"");
    }

  return ovl (args(0).round (n, mode);
}

This change will require the definition of a new octave::rounding_mode enum and new two-argument round functions in octave_base_value and any class that needs them.  Using template functions at that point may help avoid some duplication.

Doing it this way eliminates the explicit checks on data types, which is something I would like to avoid.

John W. Eaton <jwe>
Project Administrator
Fri 24 May 2019 05:24:21 PM UTC, comment #8: 

The function round is in mappers.cc.  I rewrote the first bit to

  if (nargin == 1)
    return ovl (args(0).round ());

which exactly preserves the functionality we have today where the round() function is a method on an octave_value object.

I then tried the following which works, but seems kind of slow because I am not doing an in-place map operation on the object x.

octave_value_list retval (1);

if (args(0).is_double_type ())
  {
    if (args(0).isreal ())
      {
        Matrix x = args(0).matrix_value ();
        x *= pow (10.0, N);
        x = x.map<double> (octave::math::round);
        x /= pow (10.0, N);
        retval(0) = octave_value (x);

I can work around this by writing my own static function in mappers.cc.

template <typename T>
void
do_map_round (Array<T>& x)
{
  for (octave_idx_type i = 0; i < x.numel (); i++)
    x.xelem (i) = octave::math::round (x.xelem (i));
}

followed by

octave_value_list retval (1);

if (args(0).is_double_type ())
  {
    if (args(0).isreal ())
      {
        NDArray x = args(0).array_value ();
        x *= pow (10.0, N);
        do_map_round (retval);
        x /= pow (10.0, N);
        retval(0) = octave_value (x);

Of course, I will have to write at least 4 if branches (double+real, double+complex, single+real, single+complex) and maybe another two branches for (sparse+real, sparse+complex).  Is there some other, more natural way, to tackle this?

A diff for the changes so far is attached.

Rik <rik5>
Project Administrator
Fri 24 May 2019 05:23:15 PM UTC, comment #7: 

Seems like some stuff got truncated.

The function round is in mappers.cc.  I rewrote the first bit to

  if (nargin == 1)
    return ovl (args(0).round ());

which exactly preserves the functionality we have today where the round() function is a method on an octave_value object.

I then tried the following which works, but seems kind of slow because I am not doing an in-place map operation on the object x.

Rik <rik5>
Project Administrator
Fri 24 May 2019 05:00:00 PM UTC, comment #6: 

Adding jwe to the CC list for some guidance on coding strategy.

This bug would be easy to resolve by writing a new m-file round.m and renaming the C++ function to something internal like _round_.  In Octave's languagage the operation to accomplish is just

Y=round(X*10^N)/10^N;

However, round is used so frequently by coders coming from Matlab that I think it is important that it be written in C++ for speed.  The input validation, written in an m-file, would be significant.

So I started down that route, but it has gotten ugly.

The function round is in mappers.cc.  I rewrote the first bit to

  if (nargin == 1)
    return ovl (args(0).round ());

which exactly preserves the functionality we have today where the round() function is a method on an octave_value object.

I then tried the following which works, but seems kind of slow because I am not doing an in-place map operation on the object x.

Rik <rik5>
Project Administrator
Tue 21 May 2019 05:14:09 PM UTC, comment #5: 

Documentation on Matlab's round function is available here (https://www.mathworks.com/help/matlab/ref/round.html).

They seem to implement the scheme suggested by Michael in comment #2.  For example, if you use the two-input form "round (x,n)" they require that x be a floating point class (single or double).  Similarly, they have an example of the possible confusion that can result when a floating point number looks like it should round in one direction, but doesn't because of extra precision.

format short
x = 112.05 - 110
x = 2.05
y = round (x, 1)
%% Expected value from above would be 2.1
y = 2.0
%% Check significant figures
format long
x
x = 2.049999999999997

So this wouldn't be that hard to implement.

We could alse decide whether we want to be nicer than Matlab and work with integer inputs when n != 0.  This would require converting to doubles, rounding, and then converting back to the original type.  Because of saturation, you could get some weird effects though.

round (uint8 (251), -2)
=> 255

Rik <rik5>
Project Administrator
Mon 11 Feb 2019 08:36:09 PM UTC, comment #4: 

It's also compatible with Python's 2-argument round function.

Added in Matlab R2014b.

Mike Miller <mtmiller>
Project Administrator
Sun 10 Feb 2019 12:27:11 AM UTC, comment #3: 

My guess is that Matlab went this way so that they could grab users of Excel.  See the documentation for the Excel function round (https://support.office.com/en-us/article/ROUND-function-C018C5D8-40FB-4053-90B1-B3E7F61A213C).  It doesn't make it right, but they were catering to peopele who probably don't know the math behind the round function.

Rik <rik5>
Project Administrator
Sat 09 Feb 2019 08:48:20 PM UTC, comment #2: 

Thank you.

The downstream bug I filed was to a matlab project that I would like to see compatible with octave. I suggestex to them they could do the same thing (and also said they could use . for multiplication and division.

Joshua Gay <josh>
Sat 09 Feb 2019 08:20:51 PM UTC, comment #1: 

What trivial problems make the guys at Mathworks feel the need to complexify their API... If you only need this to solve a given problem right now, just use

Y=round(X*10^N)/10^N;

But beware that these are then floating-point numbers, that is, if you get let's say 1.3 in this way and 2.7, it is not guaranteed that their sum is equal to a corresponding expression of 4.0 (different from proper rounding, which gives you integer-valued doubles that conform to exact arithmetics as long as you do not exceed bitmax()).

Michael Leitner <mleitner>
Sat 09 Feb 2019 01:30:37 PM UTC, original submission:  

Matlab supports three round functions.

Y = round(X)
Y = round(X,N)
Y = round(X,N,type)

It would be nice if octave could support these as well, although round(X,N) is what would help me the most right now.

I apologize if a feature request has already been filed against this -- I did spend several minutes searching.

Joshua Gay <josh>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #46967:  roundn.diff added by rik5 (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 jwe (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by mleitner (Posted a comment)
  • -email is unavailable- added by josh (Submitted the item)
  • -email is unavailable- added by josh
  •  

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

    Date Changed by Updated Field Previous Value => Replaced by
    2019-05-24 rik5 Attached File- => Added roundn.diff, #46967
        Carbon-Copy- => Added jwe
    2019-02-11 mtmiller Severity3 - Normal => 1 - Wish
        Priority5 - Normal => 3 - Low
    2019-02-10 rik5 Item GroupMatlab Compatibility => Feature Request
        StatusNone => Confirmed
        Release4.4.0 => dev
    2019-02-09 josh Carbon-Copy- => Added j.gay@ieee.org

    Back to the top


    Powered by Savane 3.5