bugGNU Octave - Bugs: bug #59636, iqr function does not produce...

 
 

bug #59636: iqr function does not produce Matlab compatible output

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Mon 07 Dec 2020 10:30:03 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
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

Wed 20 Apr 2022 10:22:48 PM UTC, comment #11: 

I'm going to close this report because the updated iqr function has been included in Octave core and released as part of Octave 7.

The question about conditional installation in the statistics package should be addressed to the package maintainer.  Offhand, it would seem that the new iqr is sufficiently good that a separately maintained version in the statistics package is not necessary.

Rik <rik5>
Group administrator
Tue 14 Dec 2021 06:29:29 PM UTC, comment #10: 

noting that this version of iqr is rolling out in Octave 7, should the statistics package's install-conditionally version be updated to match?

Nicholas Jankowski <nrjank>
Group Member
Thu 17 Jun 2021 06:17:33 PM UTC, comment #9: 

The patch in comment #7 didn't apply (maybe because of wrong line endings in your patch?).

I fixed a few typos and style issues. I also moved some conditions around to make the validation a little bit easier. I pushed the changed version to default here:
https://hg.savannah.gnu.org/hgweb/octave/rev/fdbba73edde2

Sorry, it took so long for someone to take this up.

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Fri 11 Dec 2020 08:53:59 PM UTC, comment #8: 

just checked octave and the only iqr references were 'seealso's in other statistics functions.

same for statistics toolbox except that it also contains an 'install-conditionally' version of iqr.

Nicholas Jankowski <nrjank>
Group Member
Fri 11 Dec 2020 07:54:15 PM UTC, comment #7: 

attached is a patch to replace the current iqr with one that produces matlab compatible output and is about 10x faster as well. it handles n-dimensional vectors, vector dimensions, and "all", and compatibly handles NaNs and Infs. a few BISTS added for verification.

It doesn't handle probability distribution class inputs, as Octave has no such class implemented yet. left as a TODO.

Most of the code is checks on the type of DIM specified, and permutations to handle when its a vec. that said the slowest part is the handoff to quantile which makes heavy use of repmat.

patch and function attached. patch also adds note to NEWS.

**note - i did not yet grep octave or the statistics toolbox to see if anything using the old iqr has BISTS that break from changing to matlab compatible quantile.

(file #50442, file #50443)

Nicholas Jankowski <nrjank>
Group Member
Fri 11 Dec 2020 06:44:25 PM UTC, comment #6: 

There is bug #58089 for sum.

Guillaume <gyom>
Fri 11 Dec 2020 06:42:24 PM UTC, comment #5: 

I remember we made a list of the functions that now take "all" as a dimension. It seemed like a fairly easy add to any one function, but tedious to find and go through them all. (i think most amount to fn(x) -> fn(x(:))

I have a draft updated version of iqr that uses the same quantile calculation as matlab (i saw that the quantile function actually allows for different methods and notes the version used by matlab, which makes all the iqr functions match as well. so this version uses that).  it also adds all the other input options expect for the probability distrtibuton objects, since Octave doesn't have that implemented yet.

will upload shortly after finishing the docstring.

Nicholas Jankowski <nrjank>
Group Member
Fri 11 Dec 2020 06:29:10 PM UTC, comment #4: 

If I understand correctly, arrays for the DIM argument in functions like sum, prod, etc., is a relatively new addition to Matlab (2018b, apparently).  If we don't already have one, maybe there should be a separate bug report for that?  I suspect that having that feature for basic core functions will help in implementing others that accept DIM arguments.

As documented in the release notes for 2018b, the list of functions changed to handle multiple dimensions at once were all, any, bounds, max, mean, median, min, mode, prod, std, sum, and var.  The size function was apparently not changed until 2019b.

John W. Eaton <jwe>
Group administrator
Tue 08 Dec 2020 01:42:49 AM UTC, comment #3: 

taking a peek at the iqr function, it dives down into quite a large number of function calls to statistics functions for what should be a fairly simple operation.  some quick tictoc tests with large arrays showed calling iqr takes about 1000x longer than the 1-line code below.

If anyone knows if that level of complexity is needed, let me know. matlab does have a subset of iqr inputs that specifically call out working with probability distributions, which it seems all of those called functions in iqr are more appropriate for. It would be good to know if 'fixing' iqr to be compatible is going to break a bunch of other stuff.

In the meantime I'll code up a compatible, faster iqr function and see if I can work in any of the other input options.

Nicholas Jankowski <nrjank>
Group Member
Tue 08 Dec 2020 01:22:53 AM UTC, comment #2: 

just adding on, I misread the input order for diff. the following produces a fairly matlab compatible iqr for normal array inputs:



function z = iqr(x,dim)
  z = diff(prctile(x,[25 75],dim),[],dim)
endfunction


Nicholas Jankowski <nrjank>
Group Member
Tue 08 Dec 2020 12:44:43 AM UTC, comment #1: 

simpler example:

Octave 5.2.0 (after peeking, the function appears unchanged in dev):

>> iqr(0:3)
ans =  2
>> iqr(0:4)
ans =  2
>> iqr(0:5)
ans =  3


Matlab 2020b:

>> iqr(0:3)
ans =
     2
>> iqr(0:4)
ans =
    2.5000
>> iqr(0:5)
ans =
     3


some websearching seems to indicate that there can be different arbitrary choices for definitions of how to calculate quartiles, and how/whether to interpolate when it doesn't land on an exact value. 

according to the Matlab help for iqr, it is "the difference between the 75th and 25th percentiles of X" and it does have the prctile function mentioned in the seealso.

if I do manual iqr:


abs(prctile(x,25,dim)-prctile(x,75,dim))


I get the same output from both octave and matlab, matching matlab, and working for n-dimensions.

for a vector  abs(diff(prctile(x,[25 75]))) works too and is probably faster, but would have to figure out the right dimensions for prctile and diff for n-dim arrays.

Nicholas Jankowski <nrjank>
Group Member
Mon 07 Dec 2020 10:30:03 PM UTC, original submission:  

iqr includes some input options that Octave does yet support (the "all" flag, an array for 'dim', probability distribution inputs).  but even apart from those,  the output on regular data appears not to match when there's an even number for median.  (note that median matches, so it's not an error stemming from that).

Eg:


>> X = reshape(1:12,[3 4])

X =

     1     4     7    10
     2     5     8    11
     3     6     9    12

>> median(X)

ans =

     2     5     8    11

>> median(X,2)

ans =

    5.5000
    6.5000
    7.5000

>> iqr(X)

ans =

    1.5000    1.5000    1.5000    1.5000

>> iqr(X,2)

ans =

     6
     6
     6


and in Octave 5.2.0


>> X = reshape(1:12,[3 4])
X =

    1    4    7   10
    2    5    8   11
    3    6    9   12

>> median(X)
ans =

    2    5    8   11

>> median(X,2)
ans =

   5.5000
   6.5000
   7.5000

>> iqr(X)
ans =

   2   2   2   2

>> iqr(X,2)
ans =

   6
   6
   6


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 #50442:  replace_iqr.diff added by nrjank (14KiB - application/octet-stream - update iqr for matlab compatibility)
file #50443:  iqr.m added by nrjank (10KiB - text/plain - update iqr for matlab compatibility)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by nrjank
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by gyom (Posted a comment)
  • -email is unavailable- added by jwe (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
    2022-04-20 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2021-12-14 nrjank Carbon-Copy- Added nir_krakauer
    2021-10-06 nrjank Dependencies- bugs #55765 is dependent
    2021-06-17 mmuetzel StatusNone Ready For Test
        Release5.2.0 dev
    2020-12-11 nrjank Attached File- Added replace_iqr.diff, #50442
        Attached File- Added iqr.m, #50443

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code