bugGNU Octave - Bugs: bug #65478, hist produces nonconformtant...

 
 

bug #65478: hist produces nonconformtant arguments error when input has ndims > 2

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Mon 18 Mar 2024 03:07:41 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  10.1.0
* Mandatory Fields

Post a Comment

Add a New Comment Rich Markup
   

Discussion

Tue 19 Mar 2024 10:13:59 PM UTC, comment #5: 

I reviewed the patch and made a few small changes.

1) "n-D" -> "N-D"
2) remove eps tolerance in BIST assert.  N-D arrays should be exactly the same as 2-D arrays.
3) Removed leading directories on filenames in commit message.

I checked it in here https://hg.savann ... /rev/67480f3843c7.

On the stable branch I placed an input validation check on Y which emits an error for N-D arrays.  See https://hg.savann ... /rev/45b7805c2a67.

Marking as Fixed and closing report.

Rik <rik5>
Group administrator
Tue 19 Mar 2024 09:36:55 PM UTC, comment #4: 

I agree that changing behavior on stable, when it is not an absolute error, should be avoided.

I like the idea of changing stable to catch the ndims>2 case and issuing an error.  Silently producing incorrect results is very bad.

Rik <rik5>
Group administrator
Tue 19 Mar 2024 02:10:43 AM UTC, comment #3: 

Question:  the patch is written against the default branch as it changes function behavior.  Should a patch to stable be made that catches ndims>2 and gives an error that hist only supports 2d inputs? Or as this fix corrects an unexpected (?) error, should it be applied to stab

The current unpatched output depending on value of n is either a meaningless error message or an silently incorrect answer that ignores anything not in the first two dimensions. 


Nicholas Jankowski <nrjank>
Group Member
Mon 18 Mar 2024 09:37:32 PM UTC, comment #2: 

patch against default attached that flattens nD inputs to 2D, adds BISTs, and updates the news.

(file #55861)

Nicholas Jankowski <nrjank>
Group Member
Mon 18 Mar 2024 08:57:58 PM UTC, comment #1: 

octave's hist has different codepaths for small and large n.  for larger in it doesn't error, but the output is incorrect:



>> [a,b] = hist ([1:5;2:6;3:7]', 15)
a =

   1   0   0
   0   0   0
   1   1   0
   0   0   0
   0   0   0
   1   1   1
   0   0   0
   1   1   1
   0   0   0
   0   0   0
   1   1   1
   0   0   0
   0   1   1
   0   0   0
   0   0   1

b =

   1.2000
   1.6000
   2.0000
   2.4000
   2.8000
   3.2000
   3.6000
   4.0000
   4.4000
   4.8000
   5.2000
   5.6000
   6.0000
   6.4000
   6.8000

>> [a,b] = hist (permute([1:5;2:6;3:7]',[1 3 2]), 15)
a =

   1
   0
   1
   0
   0
   1
   0
   1
   0
   0
   1
   0
   0
   0
   0

b =

   1.2000
   1.6000
   2.0000
   2.4000
   2.8000
   3.2000
   3.6000
   4.0000
   4.4000
   4.8000
   5.2000
   5.6000
   6.0000
   6.4000
   6.8000


Nicholas Jankowski <nrjank>
Group Member
Mon 18 Mar 2024 03:07:41 AM UTC, original submission:  

in Matlab 2023b,

[a,b] = hist([1:5;2:6;3:7;4:8]')

and

[a,b] = hist(permute([1:5;2:6;3:7;4:8]',[1 3 2]))


produce the same output:



a =

     1     0     0     0
     1     1     0     0
     1     1     1     0
     0     0     0     0
     1     1     1     1
     1     1     1     1
     0     0     0     0
     0     1     1     1
     0     0     1     1
     0     0     0     1


b =

    1.3500
    2.0500
    2.7500
    3.4500
    4.1500
    4.8500
    5.5500
    6.2500
    6.9500
    7.6500


  (it behaves as if all columns are reshaped into dim2)


Octave produces an unexpected error:

>> [a,b] = hist(permute([1:5;2:6;3:7;4:8]',[1 3 2]))
error: =: nonconformant arguments (op1 is 1x1, op2 is 1x1)
error: called from
    hist at line 204 column 20


The matlab docs do describe the input x to hist as being a vector or matrix. so supporting x being an array is undocumented, but hist in general has been around and unchanged in matlab for quite some time.  they've not created an object oriented `histogram` they recommend for use over hist, that octave has not yet implemented, and that function accepts n-D arrays for input. 

It seems like it shouldn't be too difficult for hist to also handle n-D arrays.  At a minimum, if that is discouraged, it should give a meaningful error message.

Nicholas Jankowski <nrjank>
Group Member

 

Attached Files

Attached Files
file #55861:  hist_nDinputs_bug65478.patch added by nrjank (2.5KiB - application/octet-stream)

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

Attach Files:
   
   
Comment:
   

 

Dependencies

This item does not depend on any other items.

No items depend on this one.

 

Mail Notification Carbon-Copy List

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

    Votes

    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.

     

    History

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-03-19 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2024-03-18 nrjank Attached File- Added hist_nDinputs_bug65478.patch, #55861
        StatusNone Patch Submitted
        Planned ReleaseNone 10.1.0

    Back to the top

    Powered by Savane 3.16.
    Corresponding source code