bugGNU Octave - Bugs: bug #59466, [octave forge] (communications)...

 
 

bug #59466: [octave forge] (communications) de2bi improper input handling

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Mon 16 Nov 2020 12:40:05 PM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Nicholas Jankowski Open/Closed:  * Closed
Release:  * 5.2.0 Operating System:  * Microsoft Windows
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Tue 14 Mar 2023 03:30:58 AM UTC, comment #4: 

closing bug as per comment #2

Nicholas Jankowski <nrjank>
Group Member
Wed 29 Dec 2021 12:36:50 PM UTC, comment #3: 

Can this bug can be closed?

nir_krakauer added the patch and the newest communications package  v 1.2.3 was released on 2021-04-02

Anonymous
Thu 18 Mar 2021 08:08:12 PM UTC, comment #2: 
Nir Krakauer <nir_krakauer>
Mon 16 Nov 2020 07:00:14 PM UTC, comment #1: 

ok.  wrote up a quick patch that that redoes the input handling to fix the 'MSB with only two inputs' bug, it now allows empty inputs for n and/or p. It also happens to be compatible with matlab behavior where the MSB flag can occur in position 2, 3, or 4. 

I left in n being any value, including 0 or negative, but added an input check on p.  It could previously be any number, but after checking the p=0 and p=1 cases just produce errors or erroneous output depending on other options chosen, and while negative p values have a mathematical meaning in negative-base expansion, the output wasn't following those rules.  So I added a p>1 check. Added BISTS for the changes, seems to pass them all now.

I looked at bi2de, but with only 3 inputs, it seemed to have it all covered, so didn't see any need for changes. 

patch and modified m-file for di2be attached. also made a note in the NEWS file.


(file #50285, file #50286)

Nicholas Jankowski <nrjank>
Group Member
Mon 16 Nov 2020 12:40:05 PM UTC, original submission:  

as pointed out in a recent question over on Stack overflow [1], the de2bi function in the Communications package produces errors for inputs that should be handled without error, and that don't produce errors in Matlab. Unsure whether this is a change to Matlab input handling to be more flexible, but currently results in incompatible code and an unexpected warnings.

details:

de2bi takes up to 4 inputs:

de2bi(decimal_input,num_columns,base_for_conversion,msb_flag)

the inputs must be in order, but the msb flag as the only non-numeric input can be 2nd, 3rd, or 4th (nothing may follow it).  the first non-string input after the decimal input is assumed to be the number of output columns, so specifying the base requires columns also be specified.  Matlab allows you to use [] as a placeholder for either number of columns or base.

e.g., on Matlab 2020a:

>> de2bi(19)
ans =
     1     1     0     0     1

>> de2bi(19,'left-msb')
ans =
     1     0     0     1     1

>> de2bi(19,3,'left-msb')
Error using de2bi (line 162)
Specified number of columns in output matrix is too small.

>> de2bi(19,5,'left-msb')
ans =
     1     0     0     1     1

>> de2bi(19,[],'left-msb')
ans =
     1     0     0     1     1

>> de2bi(19,[],[],'left-msb')
ans =
     1     0     0     1     1

>> de2bi(19,[],5,'left-msb')
ans =
     3     4


However, on Octave 5.2.0 with Communications 1.2.2:



>> de2bi(19)
ans =
   1   1   0   0   1

>> de2bi(19,'left-msb')
error: ones: invalid data type specified
error: called from
    de2bi at line 79 column 5

>> de2bi(19,3,'left-msb')
ans =
   0   1   1

>> de2bi(19,3,'right-msb')
ans =
   1   1   0

>> de2bi(19,5,'left-msb')
ans =
   1   0   0   1   1

>> de2bi(19,[],'left-msb')
ans =
   1   0   0   1   1

>> de2bi(19,[],[],'left-msb')
ans = [](0x0)

>> de2bi(19,[],5,'left-msb')
ans =
   3   4


I don't think it's an issue that Octave allows specifying fewer columns than needed to fit the whole number, but it looks like the rest could be fixed by revisiting the input parsing for the function. 

will take a look later today.

[1] https://stackoverflow.com/a/64857872/4598449


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 #50285:  di2be_input_patch.diff added by nrjank (4KiB - application/octet-stream - modified de2bi with corrected input handling, and diff patch to implement in package)
file #50286:  de2bi.m added by nrjank (3KiB - text/plain - modified de2bi with corrected input handling, and diff patch to implement in package)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by nir_krakauer (Posted a comment)
  • -email is unavailable- added by nrjank (Submitted the item)
  • -email is unavailable- added by nrjank (package maintainer)
  •  

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

    Date Changed by Updated Field Previous Value => Replaced by
    2023-03-14 nrjank StatusNone Fixed
        Open/ClosedOpen Closed
    2020-11-16 nrjank Attached File- Added di2be_input_patch.diff, #50285
        Attached File- Added de2bi.m, #50286
    2020-11-16 nrjank Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code