bugGNU Octave - Bugs: bug #65714, "hist" throws error when...

 
 

bug #65714: "hist" throws error when input numbers are very close

Submitter:  Hartmut <hardy>
Submitted:  Wed 08 May 2024 02:29:12 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Ready For Test Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * 9.1.0 Operating System:  * Any
Fixed Release:  None Planned Release:  9.2.0 (current stable)
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 19 May 2024 02:41:51 AM UTC, comment #9: 

now pushed to stable as

https://hg.savannah.gnu.org/hgweb/octave/rev/ca190a4a366c

with an additional test for both instances of that bug. leaving as ready for test for now.

Nicholas Jankowski <nrjank>
Group Member
Sun 19 May 2024 01:00:38 AM UTC, comment #8: 

thanks, that's what i get for going back and forth between this and the bar patches. 

will push it to stable as originally intended.

Nicholas Jankowski <nrjank>
Group Member
Sat 18 May 2024 12:02:32 PM UTC, comment #7: 

The code itself is fine, but the patch is on default, not stable as indicated by comment #4 and by the edit to NEWS.9 instead of NEWS.10.

Arun Giridhar <arungiridhar>
Group Member
Sat 18 May 2024 02:52:49 AM UTC, comment #6: 

pushed the patch as

https://hg.savannah.gnu.org/hgweb/octave/rev/e8e951bcf701

marking as ready for test

Nicholas Jankowski <nrjank>
Group Member
Fri 10 May 2024 07:24:24 AM UTC, comment #5: 

I have just succesfully tested Nicholas' patch from comment #4:

  • My original "bug code" (comment #0) does not throw an error any more.
  • All tests in hist.m pass (also the 2 new tests from the patch).


I can also follow Nicholas' reasoning for this solution: Since, for small differences between data points, we cannot perfectly mimic Matlab's behavior (and Matlab's behavior is not beautiful in this case either), the best we can do is to not error out and to present some useful histogram figure to the user of our hist.m. Both is now done with this patch.

Hartmut <hardy>
Fri 10 May 2024 02:50:02 AM UTC, comment #4: 

Attached patch against stable avoids the bug and adds a couple BISTs to test when range(Y) < 20eps. In that case it just uses the max-min = 0 calculation for bins.  This means that for small Y range they will all lump together in a single bin.  It doesn't match matlab, but it can't, since matlab can plot things with double resolution.  This at least avoids the condition that is currently erroring.  If there's some concern that the arbitrary choice of 20*eps is too large, maybe it could be set to something closer to <16*eps where the error occurs, but I thought this margin would be better since I don't know if there could be machine-to-machine variation.

(file #56030)

Nicholas Jankowski <nrjank>
Group Member
Fri 10 May 2024 01:51:42 AM UTC, comment #3: 

oh and the error occurs for anything smaller than 1+15*eps

Nicholas Jankowski <nrjank>
Group Member
Fri 10 May 2024 01:46:08 AM UTC, comment #2: 

ok, so when setting bins for x, it checks:



    if (min_val != max_val)
      x = 1:2:2*n;
      x = ((max_val - min_val) * x + 2*n*min_val) / (2*n);
    else
      x = (-floor ((n-1)/2):ceil ((n-1)/2)) + min_val;
    endif


for [1, 1+0.1*eps], min_val == max_val, so it does the else.  for 1+eps it sees min_val != max_val, so it tries to do the math to figure what the x's should be.

even in matlab the behavior of the min/max values for x are ... strange for small y values like this. It always does two bars of height 1, with bar width and separation and xlim related somehow to the spacing between the values. (matlab obviously has plotting values that support doubles). We probably don't need to perfectly emulate this corner case, but it doesn't error, and we should try not to do that.

Nicholas Jankowski <nrjank>
Group Member
Thu 09 May 2024 09:23:15 PM UTC, comment #1: 

the input to bar for the two cases for the line:

bar (hax, x, freq, "hist", varargin{iarg:end});

for the first case:
x = -3:1:6
freq = [0 0 0 0 2 0 0 0 0 0]'


for the second case it has:

x = ones (10, 1);
freq = [1 0 0 0 1 0 0 0 0 0]'

the latter x is what's triggering the error.  need to see what hist is doing to generate that from your input.

Nicholas Jankowski <nrjank>
Group Member
Wed 08 May 2024 02:29:12 PM UTC, original submission:  

I observed this akward behavior of the "hist" function in Octave. When I run this (artificially simplified demo) code:


clear
close all

figure
hist([1, 1+0.1*eps]) % -> OK

figure
hist([1, 1+eps])     % -> throws error in Octave

Then the first "hist" command gives me a reasonable histogram figure, but the second call to "hist" throws the following error:


error: bar: X vector values must be unique
error: called from
    __bar__ at line 70 column 9
    bar at line 117 column 18
    hist at line 249 column 5
    bug_hist at line 8 column 1


I know that I cannot expect a beautiful histogram figure from these input numbers. But a similar scenario happend with my "real numbers" as well. And what I expected in this case is a histogram that shows me that all my numbers are very close to each other. The current Octave behavior, to only throw an error and exit the m-script, is not very useful in this real life situation.

In addition, Matlab produces a histogram figure also for the second part of my above sample script, it does not throw an error in this situation.

I know that the OpenGL plotting has a problem with limited resolution in plotting, maybe this is the deeper reason for the above behavior. But even in this case, I think that our hist command should then do just some rounding (and just discard differences that a smaller than eps maybe, that could not be plotted anyways) before calling the bar command, in order to avoid this nasty erroring.

Hartmut <hardy>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #56030:  hist_smallYs_bug65714.patch added by nrjank (3KiB - application/octet-stream - comment #3 patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by arungiridhar (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by hardy (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-05-19 nrjank Planned Release10.1.0 (current default) 9.2.0 (current stable)
    2024-05-18 nrjank StatusPatch Submitted Ready For Test
        Planned Release9.2.0 (current stable) 10.1.0 (current default)
    2024-05-10 nrjank Attached File- Added hist_smallYs_bug65714.patch, #56030
        StatusConfirmed Patch Submitted
        Planned ReleaseNone 9.2.0 (current stable)
    2024-05-10 nrjank StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code