bugGNU Octave - Bugs: bug #63282, Make dec2base accept negative...

 
 

bug #63282: Make dec2base accept negative and/or fractional inputs

Submitter:  Arun Giridhar <arungiridhar>
Submitted:  Thu 27 Oct 2022 07:36:34 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  arungiridhar
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  9.1.0 (current stable) Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 12 Apr 2023 08:59:24 PM UTC, comment #9: 

This problem has been fixed in the development version. The fix
will be available in the next major software release. Thank you
for your bug report.

Rik <rik5>
Group administrator
Fri 24 Feb 2023 06:00:38 PM UTC, comment #8: 

@nrjank: Thank you for testing. I've now pushed it to https://hg.savannah.gnu.org/hgweb/octave/rev/1f3f7e874203

Not sure if this is a Feature Request or Matlab Compatibility, since some edge case incompatibilities identified in this thread are fixed as a side effect. Leaving the category unchanged for now.

Marking as Ready for test.

Arun Giridhar <arungiridhar>
Group Member
Fri 24 Feb 2023 02:37:40 PM UTC, comment #7: 

just a quick check, all added tests in the comment #6 patch that aren't expected to error in matlab produce compatible output with v2022b

Nicholas Jankowski <nrjank>
Group Member
Thu 23 Feb 2023 10:35:30 PM UTC, comment #6: 

Here's a combined and self-consistent patch for dec2base, dec2bin, and dec2hex.

Please test against Matlab if you can.

I have not touched the reverse functions yet (base2dec, bin2dec, hex2dec). Those will happen over the next several months, certainly well in time for Octave 9.1.

(file #54392)

Arun Giridhar <arungiridhar>
Group Member
Thu 23 Feb 2023 02:35:59 PM UTC, comment #5: 

sorry, i meant it matches what @thibaudp was reporting. just confirming that current matlab matches what he saw in 2020b:


>> dec2hex(-3,3)

ans =

    'FFD'

>> dec2bin (-3,9)

ans =

    '111111101'




Nicholas Jankowski <nrjank>
Group Member
Thu 23 Feb 2023 02:19:39 PM UTC, comment #4: 

Interestingly enough, the patch to make dec2base accept negative numbers (the subject of this bug report before the digression about dec2bin) DOES process negative numbers correctly:


octave:10> dec2base (-3, 2, 9)    # base-2, 9-digit output
ans = 111111101
octave:11> dec2base (-3, 16, 3)   # base-16, 3-digit output
ans = FFD


If you try that with unpatched Octave you'll get an error.

Anyway, I am attaching an updated patch (no new features to dec2base compared to comment #0, just basing it against current default after 6 months.

The aim of this patch is to make dec2bin and dec2hex both use dec2base which would centralize all validation and conversion in one consistent place.

(file #54390)

Arun Giridhar <arungiridhar>
Group Member
Thu 23 Feb 2023 02:10:36 PM UTC, comment #3: 

@nrjank: clarification. Did you mean that Matlab 2022b uses left-leading zeros like Octave or does it pad with ones like Matlab 2020b?

Reading through the dec2bin.m history of why I wrote it that way, I found this comment of mine from Sep 2022: https://savannah.gnu.org/bugs/?63089#comment6


The practice of left-padding with ones for negative inputs is standard two's complement notation, but the practice of further left-padding that with zeroes is not mathematically sound at all. Is Matlab still doing that or have they fixed it to consistently left-pad with ones for all negative inputs? In any case, I've added a note to the docstring explaining what to expect.


and `help dec2bin` states:

     For negative elements of D, return the binary value of the two’s
     complement.  The result is padded with leading ones to 8, 16, 32,
     or 64 bits as appropriate for the magnitude of the input.  Positive
     input elements are padded with leading zeros to the same width.  If
     the second argument LEN exceeds that calculated width, the result
     is further padded with leading zeros, for compatibility with
     MATLAB.


So Matlab was left-padding with zeros at some point in the past? I guess they have fixed it now, so we can make ours consistent. I'll get to it in the next several days, unless someone wants to go ahead earlier.

Arun Giridhar <arungiridhar>
Group Member
Thu 23 Feb 2023 01:02:55 PM UTC, comment #2: 

Matlab 2022b produces the same output as comment #1

Nicholas Jankowski <nrjank>
Group Member
Thu 23 Feb 2023 10:44:51 AM UTC, comment #1: 

This is closely related, I did not find another issue mentionning it :


./scripts/strings/dec2hex.m:93        %!assert (dec2hex (-3, 3), "0FD")

Matlab R2020b returns "FFD"


./scripts/strings/dec2bin.m:132        %!assert (dec2bin (-3, 9), "011111101")

Matlab R2020b return "111111101"

While this does not make a lot of sens to represent signed with such bit count, it may be confusing to pad with 0 instead.

It might be different on latest version of Matlab though.

Thibaud Pigeon <thibaudp>
Thu 27 Oct 2022 07:36:34 PM UTC, original submission:  

This bug report is to track a WIP patch for dec2base and dec2bin. The main aims are the following:

  • Make dec2base accept negative integer inputs.
  • Remove the negative input code and all other input checks from dec2bin to reduce code duplication, especially duplicated code with subtle differences between functions.


The reason is that Matlab's dec2bin allows negative input so we need to do so too, but its dec2base takes only positive inputs, which requires ad hoc checks in dec2bin instead of a single self-consistent check in dec2base that would apply to all bases.

  • Optionally, make dec2hex accommodate fractional inputs (new feature, not essential for the above aims, but would be nice to have if it doesn't get in the way of anything else).


This work won't be completed in time for Octave 8, so this report is to track the activity and collect feedback so it can be ready for Octave 9.

WIP patch attached (only for dec2base so far, not for dec2bin).

Arun Giridhar <arungiridhar>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #54392:  combined.patch added by arungiridhar (18KiB - text/x-patch)
file #53915:  dec2base.patch added by arungiridhar (7KiB - 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 rik5 (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by thibaudp (Posted a comment)
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by arungiridhar (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-04-12 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 9.1.0 (current stable)
    2023-02-24 arungiridhar StatusIn Progress Ready For Test
    2023-02-23 arungiridhar Attached File#54390 Removed
    2023-02-23 arungiridhar Attached File- Added combined.patch, #54392
    2023-02-23 arungiridhar Attached File- Added dec2base.patch, #54390
    2022-11-29 jwe CategoryNone Octave Function
    2022-10-27 arungiridhar Attached File- Added dec2base.patch, #53915

    Back to the top

    Powered by Savane 3.12