bugGNU Octave - Bugs: bug #58147, Matlab 2020 compatibility:...

 
 

bug #58147: Matlab 2020 compatibility: dec2bin, dec2hex shouldn't error on negative inputs

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Thu 09 Apr 2020 04:06:08 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  3 - Low Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  Nicholas Jankowski 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

Sat 25 Apr 2020 09:36:21 PM UTC, comment #15: 

I made the same changes to dec2hex to support segative numbers.  See changeset https://hg.savannah.gnu.org/hgweb/octave/rev/f618edac2a29.

I'll file another bug report that dec2bin and dec2hex should be able to handle negative numbers below -flintmax/2.

I think that is enough to mark this bug as fixed.

Rik <rik5>
Group administrator
Sat 25 Apr 2020 08:24:32 PM UTC, comment #14: 

this and the xtests all looks consistent to me for now. Curious why the array tests were kicked over to xtests, though. i thought those worked fine with my original code.

Nicholas Jankowski <nrjank>
Group Member
Sat 25 Apr 2020 06:56:58 PM UTC, comment #13: 

I checked in a change that limits negative inputs to -flintmax /2, which at least then produces the correct result (https://hg.savannah.gnu.org/hgweb/octave/rev/ef7bc64a604b).

Rik <rik5>
Group administrator
Thu 23 Apr 2020 12:40:38 AM UTC, comment #12: 

The current Octave implementation has a problem with numbers greather than flintmax().  We should either restrict the range of inputs and issue an error if the input is outside that range, or modify the algorithm.

A test case is


dec2bin (-2^31 -1)


which should return


"1111111111111111111111111111111101111111111111111111111111111111"


, but instead returns


"1111111111111111111111111111111110000000000000000000000000000000"


The trouble is this line, I think


d(lt_zero_idx) += double (intmax ("uint64")) + 1;


intmax() for "uint64" is larger than flintmax() (2^53) for double values and so precision is lost.


Rik <rik5>
Group administrator
Thu 23 Apr 2020 12:21:15 AM UTC, comment #11: 

Matlab programmers apparently enjoy introducing inconsistencies in to their code.  According to the documentation,


If D is an array of floating-point numbers, and any element of D has a fractional part, then dec2bin truncates it before conversion. For example, dec2bin converts both 12 and 12.5 to '1100'. The truncation is always to the nearest integer less than or equal to that element.


This is equivalent to floor(), and I can verify with


dec2bin (12)   => "1100"
dec2bin (12.1) => "1100"
dec2bin (12.9) => "1100"
dec2bin (13)   => "1101"


However, for negative numbers they have an entirely different convention and use round().


dec2bin (-12)   => "11110100"
dec2bin (-12.1) => "11110100"
dec2bin (-12.9) => "11110011"
dec2bin (-13)   => "11110011"


Currently, Octave does stricter input validation than Matlab and issues an error whenever the input is not an integer.  For the moment, I think we should keep that standard and force the programmer to decide on a rounding strategy (fix, floor, ceil, round) so that the decision is explicit.


Rik <rik5>
Group administrator
Thu 23 Apr 2020 12:09:42 AM UTC, comment #10: 

I checked in Nicholas's patch on the development branch here https://hg.savannah.gnu.org/hgweb/octave/rev/5bb1c0cbb27e.

I checked in another changeset to change the documentation to use the more modern Octave format, increase performance, and add some %!xtest which are currently failing.  See https://hg.savannah.gnu.org/hgweb/octave/rev/ac3a078e688f.

Rik <rik5>
Group administrator
Fri 10 Apr 2020 08:26:16 PM UTC, comment #9: 

ok, after some modification of mike's code, borrowing a bit from dec2base to handle cells and arrays, and adding a bunch of tests, the attached passes all tests except the case mentioned in comment #8.





(file #48805)

Nicholas Jankowski <nrjank>
Group Member
Fri 10 Apr 2020 06:44:40 PM UTC, comment #8: 

that's close, and avoids the bitcmp issue, but need to +1 each one to get two's complement. 


if (d < intmin ("int64"))
  error ("out of range");
elseif (d < intmin ("int32"))
  d += double (intmax ("uint64")) + 1 ;
elseif (d < intmin ("int16"))
  d += double (intmax ("uint32")) + 1;
elseif (d < intmin ("int8"))
  d += double (intmax ("uint16"))+ 1;
elseif (d < 0)
  d += double (intmax ("uint8")) + 1;
endif


That then works for all test cases but one. looking at the bit length transitions:

>> dec2bin(-128)
ans = 10000000
>> dec2bin(-129)
ans = 1111111101111111
>> dec2bin(-2^15)
ans = 1000000000000000
>> dec2bin(-2^15-1)
ans = 11111111111111110111111111111111
>> dec2bin(-2^31)
ans = 10000000000000000000000000000000
>> dec2bin(-2^31-1)
ans = 1111111111111111111111111111111110000000000000000000000000000000
>> dec2bin(-2^63)
ans = 1000000000000000000000000000000000000000000000000000000000000000
>> dec2bin(-2^63-1)
ans = 1000000000000000000000000000000000000000000000000000000000000000


those all are correct except the (-2^31-1) case.


Matlab:
>> dec2bin(-2^31-1)
ans =
    '1111111111111111111111111111111101111111111111111111111111111111'

Octave:
>> dec2bin(-2^31-1)
ans = 1111111111111111111111111111111110000000000000000000000000000000
>>


so need to peek at what's happening there.

Also, need to adjust it to handling both arrays and cell arrays, which could include a mix of pos and neg values, complicates things a bit. Apparently Octave handling cell arrays in an extension beyond Matlab, which throws an error if d is not numeric. would probably also throw an 'if d<0' around that to avoid the checking cascade when not needed.

Nicholas Jankowski <nrjank>
Group Member
Fri 10 Apr 2020 05:55:10 PM UTC, comment #7: 

I would think something like this (untested)


if (d < intmin ("int64"))
  error ("out of range");
elseif (d < intmin ("int32"))
  d += double (intmax ("uint64"));
elseif (d < intmin ("int16"))
  d += double (intmax ("uint32"));
elseif (d < intmin ("int8"))
  d += double (intmax ("uint16"));
elseif (d < 0)
  d += double (intmax ("uint8"));
endif


Mike Miller <mtmiller>
Group Member
Fri 10 Apr 2020 03:43:07 AM UTC, comment #6: 

oops, forgot to attached the modified file.

(file #48798)

Nicholas Jankowski <nrjank>
Group Member
Fri 10 Apr 2020 03:41:42 AM UTC, comment #5: 

here's a quick stab at a version of dec2bin that returns the 2's complement. (not close to ready yet, but could use some algorithm pointers.)

before it calls dec2base, it uses:


  if (d < 0) ##only works for scalar numerical inputs.
    if (-d <= 128)
      bitlen = 8;
    elseif (-d <= 32768)
      bitlen = 16;
    elseif (-d <= 2^31)
      bitlen = 32;
    else
      ##TODO: extend bitlen to 64 for compatibility when bitcmp can handle 64bit
      bitlen = 53;
  endif

    d = bitcmp (-d, bitlen) + 1; ##returns same class as d, so ints get limited near intmax?
  endif


bitcmp is limited to 53 bits, is not matlab compatible in general, and returns the same class that is input, so run into some limits with large ints (i.e., intmax errors)

maybe there's a better way than bitcmp?

Nicholas Jankowski <nrjank>
Group Member
Fri 10 Apr 2020 02:32:38 AM UTC, comment #4: 

(whoops. missed a - )

seems like it could be fairly simply implemented by adding something like:


if (d < 0)
    d = bitcmp(-d)+1;
  endif


to the front of dec2bin. it works for small values, but need to specify the number of bits to use, as our (not matlab compatible) bitcmp defaults to 53 bits (based on flintmax) if unspecified, and that still errors out on some big numbers like intmax.

running a few tests:

seems dec2bin uses minimum bit length for pos, but minimum of 8 for neg even if you ask for less:


>> dec2bin(3)
ans =
    '11'

>> dec2bin(-3)
ans =
    '11111101'

>> dec2bin(3,3)
ans =
    '011'

>> dec2bin(-3,3)
ans =
    '11111101'

>> dec2bin(-3,7)
ans =
    '11111101'

>> dec2bin(-3,8)
ans =
    '11111101'

>> dec2bin(-3,9)
ans =
    '011111101'

>> dec2bin(-3,10)
ans =
    '0011111101'


when increasing value, it outputs in increments of 8 bits:

>> dec2bin(255)
ans =
    '11111111'
>> dec2bin(256)
ans =
    '100000000'
>> dec2bin(257)
ans =
    '100000001'

>> dec2bin(-128)
ans =
    '10000000'
>> dec2bin(-129)
ans =
    '1111111101111111'

>> dec2bin(intmax)
ans =
    '1111111111111111111111111111111'
>> dec2bin(-intmax)
ans =
    '10000000000000000000000000000001'
>> dec2bin(flintmax)
ans =
    '100000000000000000000000000000000000000000000000000000'
>> dec2bin(-flintmax)
ans =
    '1111111111100000000000000000000000000000000000000000000000000000'


Nicholas Jankowski <nrjank>
Group Member
Fri 10 Apr 2020 02:24:13 AM UTC, comment #3: 

seems like it could be fairly simply implemented by adding something like:


if (d < 0)
    d = bitcmp(-d)+1;
  endif
-verbatim
to the front of dec2bin. it works for small values, but need to specify the number of bits to use, as our (not matlab compatible) bitcmp defaults to 53 bits (based on flintmax) if unspecified, and that still errors out on some big numbers like intmax.

running a few tests:

seems dec2bin uses minimum bit length for pos, but minimum of 8 for neg even if you ask for less:
+verbatim+
>> dec2bin(3)
ans =
    '11'

>> dec2bin(-3)
ans =
    '11111101'

>> dec2bin(3,3)
ans =
    '011'

>> dec2bin(-3,3)
ans =
    '11111101'

>> dec2bin(-3,7)
ans =
    '11111101'

>> dec2bin(-3,8)
ans =
    '11111101'

>> dec2bin(-3,9)
ans =
    '011111101'

>> dec2bin(-3,10)
ans =
    '0011111101'


when increasing value, it outputs in increments of 8 bits:

>> dec2bin(255)
ans =
    '11111111'
>> dec2bin(256)
ans =
    '100000000'
>> dec2bin(257)
ans =
    '100000001'

>> dec2bin(-128)
ans =
    '10000000'
>> dec2bin(-129)
ans =
    '1111111101111111'

>> dec2bin(intmax)
ans =
    '1111111111111111111111111111111'
>> dec2bin(-intmax)
ans =
    '10000000000000000000000000000001'
>> dec2bin(flintmax)
ans =
    '100000000000000000000000000000000000000000000000000000'
>> dec2bin(-flintmax)
ans =
    '1111111111100000000000000000000000000000000000000000000000000000'


Nicholas Jankowski <nrjank>
Group Member
Thu 09 Apr 2020 11:29:31 PM UTC, comment #2: 

The error message isn't bad because there is significant ambiguity when converting a negative number to binary.  There are multiple representations and, although 2's complement is a popular one, there are other representation such as signed magnitude which make sense and might be preferable.

If Octave is going to implement this, then it needs to document the choice that was made  (2's complement).

Rik <rik5>
Group administrator
Thu 09 Apr 2020 04:10:12 PM UTC, comment #1: 

noting that the public facing help for both matlab functions explain the output:

Starting in R2020a, the dec2bin function converts negative numbers using their two's complement binary values.

For example, these calls to dec2bin convert negative numbers.

dec2bin(-1)
ans =
'11111111'
dec2bin(-16)
ans =
'11110000'


and the same for dec2hex:

dec2hex(-1)
ans =
'FF'
dec2hex(-16)
ans =
'F0'


dec2base still states that the input must be non-negative

Nicholas Jankowski <nrjank>
Group Member
Thu 09 Apr 2020 04:06:08 PM UTC, original submission:  

Matlab 2020a has the following in the release notes:


dec2bin and dec2hex Functions: Accept negative numbers as input arguments


Octave 5.2.0 currently passes both inputs to dec2base which produces an error:

>> dec2bin(-3)
error: dec2base: input must be real non-negative integers
error: called from
    dec2base at line 72 column 5
    dec2bin at line 48 column 7
>> dec2hex(-3)
error: dec2base: input must be real non-negative integers
error: called from
    dec2base at line 72 column 5
    dec2hex at line 48 column 7


in matlab 2020a, it accepts the inputs, although I don't quite follow the logic, and they seem to do some conversion before passing to dec2base, since that still rejects negative input:


>> dec2hex(-3)
ans =
    'FD'
>> hex2dec(ans)
ans =
   253
>> dec2bin(-3)
ans =
    '11111101'
>> bin2dec(ans)
ans =
   253
>> dec2base(-3,2)
Error using dec2base (line 22)


Nicholas Jankowski <nrjank>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #48805:  dec2bin.m added by nrjank (4KiB - text/plain - almost working version)
file #48798:  dec2bin.m added by nrjank (2KiB - text/plain - dec2bin with two's complement modification, v1)

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2020-04-25 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2020-04-23 rik5 Release5.2.0 dev
    2020-04-10 nrjank Attached File- Added dec2bin.m, #48805
    2020-04-10 nrjank Attached File- Added dec2bin.m, #48798
    2020-04-09 rik5 Priority5 - Normal 3 - Low
        Item GroupMatlab Compatibility Feature Request
        StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code