bugGNU Octave - Bugs: bug #49961, [octave forge] (statistics)...

 
 

bug #49961: [octave forge] (statistics) sign_test() returns incorrect p-value

Submitter:  Kirill Pushkaryov <kpushkaryov>
Submitted:  Sat 31 Dec 2016 05:53:38 AM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 01 Aug 2022 04:31:59 PM UTC, comment #7: 

signtest function has been updated and fully Matlab compatible. Tests have been added, and incorrect values have been fixed. Commit 299c639 has been pushed at https://github.com/gnu-octave/statistics/commit/299c6398d52924ff13cea7abc1be59db1d4cfa8a
The sign_test function has been deprecated and removed from next release.
This bug can be closed.

Andreas Bertsatos <pr0m1th3as>
Wed 24 Nov 2021 04:38:55 PM UTC, comment #6: 

seems both sign_test and signtest now reside only in the statistics package.

the behavior from comment #0 is still present in both functions using statistics pkg v1.4.2.  checking against Matlab 2021a's signtest (it has no sign_test), the output matches that of octave's signtest.

the comment #3 function in both programs does show discrepancy between Matlab and Octave's signtest:

Matlab 2021a:

>> [pval, h] = signtest([-ones(1, 1000) 1], 0, 'tail', 'left')
pval =
  1.0917e-218
h =
  logical
   1


Octave 6.4.0, Statistics 1.4.2:

>> [pval, h] = signtest([-ones(1, 1000) 1], 0, 'tail', 'left')
pval = 1
h = 0


seems like Kai's suggestion from comment #4 still stands:  need to fix the issues in signtest, then decide how to make any changes in sign_test, assuming the plan is to keep sign_test and not just deprecate it (didn't see any deprecate notes in NEWS)

Nicholas Jankowski <nrjank>
Group Member
Sat 21 Apr 2018 05:40:41 PM UTC, comment #5: 

Reassigning to the Octave Forge statistics package. The sign_test.m function is no longer a part of Octave and has been moved to the package.

Mike Miller <mtmiller>
Group Member
Thu 05 Jan 2017 05:15:45 PM UTC, comment #4: 

Regarding comment #2: If those functions are going to be dropped, why aren't they deprecated right now? Maybe one can add a pointer to use the statistics package.

Regarding comment #3: That makes a blind copy&past to core less desirable, if the code contains errors. I tried some the tests from the Matlab documentation ([1] in comment #1) but those examples are working. Unless Octave has a Matlab compatible rng-function (bug #42557) those tests are ugly "hand-writing" of many numbers.

I suggest to first fix the issues in `signtest` from the statistics package and then copy&paste the code to a deprecated octave core function `sign_test`.

Maybe Arno Onken as package maintainer has more insight into that issue.

Kai Torben Ohlhus <siko1056>
Group Member
Thu 05 Jan 2017 04:24:36 AM UTC, comment #3: 

As far as the sign test is not a general purpose algorithm, it sounds reasonable to leave it to the statistics package.

Unfortunately, it seems that signtest() is not entirely correct too, at least when tail = 'right':

     switch lower(tail)
        case 'both'
          w = min(sum(x<my),sum(x>my));
          pl = binocdf(w, n, 0.5);
          p = 2*min(pl,1-pl);
        case 'left'
          w = sum(x<my);
          p = binocdf(w, n, 0.5);
        case 'right'
          w = sum(x>my);
          p = 1 - binocdf(w, n, 0.5);
        otherwise


Here we have p = 1 - binocdf(w, n, 0.5) = 1 - P(X <= w) = P(X > w), so X = w is omitted. But p-value is by definition the probability to observe the same or more extreme value under H0.

More problems I've noticed with signtest():
1) ties aren't excluded
2) incorrect results with tail = 'left':

>> [pval, h] = signtest([-ones(1, 1000) 1], 0, 'tail', 'left')
pval =  1
h = 0


We checked if a vector of a thousand -1's and one 1 has a median less than 0, and signtest() rejected this hypothesis! In the source we have p = binocdf(w, n, 0.5) = P(X <= sum(x < 0)). So, the more components of x are less than 0, the closer p is to 1, but it should be the other way around.

@Kai Torben Ohlhus: Alas, I don't have any tests. I've found a paper on the sign test (Dixon, Wilfrid J., and Alexander M. Mood. "The statistical sign test." Journal of the American Statistical Association 41.236 (1946): 557-566), and it includes a table of critical values, but that wouldn't be convenient for tests. I suppose, the needed values could be generated using some trustworthy statistics software.

Kirill Pushkaryov <kpushkaryov>
Tue 03 Jan 2017 04:17:34 PM UTC, comment #2: 

At the next release of Octave the excess statistics functions are going to be dropped from core and moved to the statistics package.  For the time being, we could copy "signtest" to "sign_test" on the stable branch.  How does that sound?

Rik <rik5>
Group administrator
Tue 03 Jan 2017 01:01:48 AM UTC, comment #1: 

Thank you for the report. The function `sign_test` hasn't been worked on for years and is in contrast to `signtest` from the statistics package more Matlab compatible [1]. To improve the situation, I see the options:

a) Replace the current `sign_test` by `signtest` from the statistics package.

b) Fix `sign_test` by an expert of that area.

Personally, I favor option a), as b) might take a long time to happen.

@Kirill Pushkaryov: Can you add some references to known tests for that function?

[1]: https://www.mathworks.com/help/stats/signtest.html

Kai Torben Ohlhus <siko1056>
Group Member
Sat 31 Dec 2016 05:53:38 AM UTC, original submission:  

The function sign_test() from core Octave (ver. 4.0.0) returns a seemingly incorrect p-value.
In Octave command window:

>> x = [1 1 1 -1 -1]
x =

   1   1   1  -1  -1

>> [pval, b, n] = sign_test(x, -x)
pval =  0.37500
b =  3
n =  5


At the same time, signtest() function from the statistics package (ver. 1.2.4) gives a different result:

>> pval2 = signtest(x, -x)
pval2 =  1.00000


SciPy's scipy.stats.binom_test(3, 5) returns 1.0 too.

I inspected the source code of sign_test() and believe that it improperly handles the boundary points. In sign_test.m we have:

  n   = sum (x != y);
  b   = sum (x > y);
  cdf = binocdf (b, n, 1/2);
...
  if (strcmp (alt, "!=") || strcmp (alt, "<>"))
    pval = 2 * min (cdf, 1 - cdf);
  elseif (strcmp (alt, ">"))
    pval = 1 - cdf;
  elseif (strcmp (alt, "<"))
    pval = cdf;
  else
    error ("sign_test: option %s not recognized", alt);
  endif

By default alt = "!=". So, mathematically cdf = F(b) = P(X <= b) and 1 - cdf = P(X > b), where X has binomial distribution with parameters n, 0.5. Depending on whether cdf < 1 - cdf (i.e. cdf < 0.5) or cdf > 0.5 we have pval = 2 P(X <= b) or pval = 2 P(X > b). The first case correctly includes X = b, but the second does not. A similar problem will occur with alt = ">".

sign_test.m in Octave 4.2.0 contains the same code.

Kirill Pushkaryov <kpushkaryov>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #39335:  sign_test_bug.m added by kpushkaryov (203B - text/x-objcsrc - Demonstration code)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by pr0m1th3as (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by siko1056 (statistics package maintainer)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by siko1056 (Posted a comment)
  • -email is unavailable- added by kpushkaryov (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-08-02 siko1056 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2018-04-21 mtmiller CategoryOctave Function Octave Package
        Release4.0.0 dev
        Summarysign_test() returns incorrect p-value [octave forge] (statistics) sign_test() returns incorrect p-value
    2017-01-05 siko1056 Carbon-Copy- Added -email is unavailable-
    2017-01-03 siko1056 StatusNone Confirmed
    2016-12-31 kpushkaryov Attached File- Added sign_test_bug.m, #39335

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code