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
   

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>
Group 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>
Group 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>
Group administrator
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>
Group 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>
Group administrator
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>
Group administrator
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>
Group 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 group 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.13-4448.
    Corresponding source code