bugGNU Octave - Bugs: bug #62947, nextpow2 gives incorrect result...

 
 

bug #62947: nextpow2 gives incorrect result for 0.5 <= x < 1

Submitter:  Marco Caliari <caliari>
Submitted:  Wed 24 Aug 2022 07:46:46 AM UTC
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 7.2.0 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Wed 24 Aug 2022 04:27:23 PM UTC, comment #6: 

I'd just note that for the original code with

if (f == 0.5)
  n = n - 1;
endif

is not the same as

n(f == 0.5)--

unless the input X is a scalar.  For example, for X = [NaN, Inf, 3, 56, 64], the original code returned [0, 0, 2, 6, 7] but with logical indexing instead of the incorrect IF statement, the result is [0, 0, 6, 6].

So the original made the mistake of thinking in scalar terms.

John W. Eaton <jwe>
Project Administrator
Wed 24 Aug 2022 04:05:57 PM UTC, comment #5: 

Thanks for the quick fix.  This entire bug was reported and fixed in approximately 8 hours.

I made one small change which was to tag the BIST tests that are specific to this bug report.  See http://hg.savannah.gnu.org/hgweb/octave/rev/075443476dfb.

For the history, you could try "hg log -f nextpow2.m".  The first interesting changeset seems to be back in 2018.

changeset:   25236:69b21b8a0e9f
user:        Rik <rik@octave.org>
date:        Thu Apr 12 14:34:10 2018 -0700
summary:     nextpow2.m: Compute value correctly for Inf or NaN inputs (bug #53463).

and the change was

   [f, n] = log2 (abs (x));
-  n(f == 0.5)--;
+  idx = (n == 0);   # Find any failures of log2 function (n == 0)
+  n(idx) = f(idx);  # and copy over value.
+  n(f == 0.5)--;

Otherwise, you can go back to the initial code in 1996

changeset:   2539:1dca28c213f0
user:        jwe
date:        Tue Nov 19 23:55:06 1996 +0000
summary:     [project @ 1996-11-19 23:54:48 by jwe]

+  [f, n] = log2 (abs (x));
+  if (f == 0.5)
+    n = n - 1;
+  endif

Apparently, it has just always been this way.

Marking bug as fixed and closing report.

Rik <rik5>
Project Administrator
Wed 24 Aug 2022 03:21:57 PM UTC, comment #4: 

Thanks for reviewing.

I pushed the change with some added BISTs to the stable branch:
https://hg.savannah.gnu.org/hgweb/octave/rev/863730dd0f83

I'd still be interested to know why this was implemented differently before. I hope the new implementation doesn't break in other corner cases. But all BISTs still pass for me at least...

Markus Mützel <mmuetzel>
Project Member
Wed 24 Aug 2022 02:51:17 PM UTC, comment #3: 

This is rather amazing that a bug like this has been present in Octave for years, probably decades.

I can confirm with Matlab as well that input of 0.5 should result in -1 and input of 0.6 should result in 0.

This is a numerical error and should be fixed as soon as possible so I have changed the Release field from "dev" to "7.2.0".

I reviewed the two implementations and I would prefer the one in comment #1.  Because the output vector is never changed in size there is no advantage to initializing it with "zeros (size (x))" before using it.

@Markus: Can you make a changeset from comment #1 and check it in on the stable branch?  It should probably have BIST tests for inputs of 0.5 and 0.6.

Rik <rik5>
Project Administrator
Wed 24 Aug 2022 08:09:08 AM UTC, comment #2: 

Sorry. Misread your suggestion. It is equivalent (and probably slightly better) to what I wrote...

Markus Mützel <mmuetzel>
Project Member
Wed 24 Aug 2022 08:07:42 AM UTC, comment #1: 

I can confirm this issue.

The "dump" approach would be to implement this function as the following:

function n = nextpow2 (x)

  n = ceil (log2 (abs (x)));
  n(x==0) = 0;

endfunction

Is there a reason why it is implemented differently at the moment?

Markus Mützel <mmuetzel>
Project Member
Wed 24 Aug 2022 07:46:46 AM UTC, original submission:  

The following

nextpow2(0.5)

gives -0.5, while it should be -1. And

nextpow2(0.6)

gives 0.6 itself, while it should be 0 (and the same for any other value x, 0.5 < x < 1). What about something like

n = zeros (size (x));
idx = (x != 0);
n(idx) = ceil (log2 (abs (x(idx))));

Marco Caliari <caliari>
Project Member

 

(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 jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by caliari (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 project members can vote.

     

     

     

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-08-24 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2022-08-24 mmuetzel StatusPatch Reviewed Ready For Test
    2022-08-24 rik5 StatusConfirmed Patch Reviewed
        Releasedev 7.2.0
    2022-08-24 mmuetzel StatusNone Confirmed

    Back to the top


    Powered by Savane 3.9