bugGNU Octave - Bugs: bug #64555, corr produces input validation...

 
 

bug #64555: corr produces input validation error with two inputs having different number of columns

Submitter:  None
Submitted:  Tue 15 Aug 2023 10:48:24 AM UTC
   
 
Category:  None Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * dev
Operating System:  * Any Fixed Release:  9.1.0
Planned Release:  9.1.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 12 Jan 2024 09:53:59 PM UTC, comment #9: 

Marking as Fixed and closing report.

Rik <rik5>
Group administrator
Thu 17 Aug 2023 04:26:25 PM UTC, comment #8: 

i think one of the failures was stepwisefit, so that would make sense.

Nicholas Jankowski <nrjank>
Group Member
Thu 17 Aug 2023 03:05:36 PM UTC, comment #7: 

I think this fixed one of the failing test in "statistics".
There are 3 FAILures and used to be 4.
No problem with "signal".

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Thu 17 Aug 2023 02:49:17 PM UTC, comment #6: 

ok, had to stop relying on cov for input validation and put what little of that there is into corr. handling of row vects and >2d ahead of call to cov allowed too much past cov's validation.

pushed fix to default as http://hg.savannah.gnu.org/hgweb/octave/rev/4f287cb8002c

added an extra check for >2D inputs. added input validations similar to cov. added tests to verify proper output for inputs with different numbers of columns and for proper NaN output shape for various inputs, and input validation tests. noted the 'same number of rows' requirement in the docstring. also updated cov BISTs that missed some Y checks and fixed varargin check that was missing logical opt.  passes all tests and a make check against core octave.

marking ready for test. "eagerly" awaiting more identified corr/cov issues from package tests.

Nicholas Jankowski <nrjank>
Group Member
Wed 16 Aug 2023 02:06:31 PM UTC, comment #5: 

that definitely solves the original problem.  but it removes the check for dim>3 mismatches, and it turns out matlab handling of nD arrays is odd. stated invalid unless one input is scalar.  but it also takes a row vector.  either case produces an array of NaNs but the shape seems odd. will see if there's a logical way to tease that out. note though, that it'll hit the call to cov before rik's earlier fix for row vectors, which we left in here so cov would handle all input checks and avoid duplication.  might need to get away from that with all these separate cases.

Nicholas Jankowski <nrjank>
Group Member
Wed 16 Aug 2023 01:41:03 PM UTC, comment #4: 

correction.  input error on my part. it seems changing the size to a row check does fix the error. will run a few more checks to be sure. 



>> corr(rand(13,4),rand(13,1))
ans =

   0.2624
   0.2377
   0.2568
   0.1727

>> corr(rand(13,4),rand(13,2))
ans =

   0.712223   0.148490
  -0.401667  -0.400632
   0.068554   0.174803
  -0.262668  -0.220688


Nicholas Jankowski <nrjank>
Group Member
Wed 16 Aug 2023 01:28:59 PM UTC, comment #3: 

ok, the input check in corr is wrong.  a size check was put in mainly to catch mixed row/column vector inputs which should error in corr but won't in cov.  it should be a row count comparison (which will also catch the mixed vector orientation)

however, just changing the input check to:


if (rows (x) != rows (y))
  error ("corr: X and Y must have the same number of rows");
endif


the same inputs now produce a cov error:


>> cov(rand(13,4),rand(13,1))
error: cov: X and Y must have the same number of observations
error: called from
    cov at line 191 column 9


that cov behavior is matlab compatible.  so will need to figure out how to modify corr to pass something with unequal column count to cov and extract the right results.

Nicholas Jankowski <nrjank>
Group Member
Wed 16 Aug 2023 01:14:31 PM UTC, comment #2: 

I haven't had a chance to dig into this one yet, but i can confirm with the default version of corr and it's not related to the statistics function.


the critical line is the call to stepwisefit is on line 94:

corr(X(:,~X_inds),r)

the comment #0 test code creates the inputs as:


>> X(:,~X_inds)
ans =

    7   26    6   60
    1   29   15   52
   11   56    8   20
   11   31    8   47
    7   52    6   33
   11   55    9   22
    3   71   17    6
    1   31   22   44
    2   54   18   22
   21   47    4   26
    1   40   23   34
   11   66    9   12
   10   68    8   12

>> r
r =

    78.500
    74.300
   104.300
    87.600
    95.900
   109.200
   102.700
    72.500
    93.100
   115.900
    83.800
   113.300
   109.400


and the old version of corr returned a 4x1 array:


corr(X(:,~X_inds),r)
ans =

   0.7307
   0.8163
  -0.5347
  -0.8213


so a corr test case for the same would be:


corr(rand(13,4),rand(13,1))

ans =

  -0.608954
  -0.072938
   0.150300
   0.022153


triggering the error you see below.  you're correct that this should work, the current version of matlab corr produces the expected 4x1 output.  probably just faulty input validation logic.  will take a look.

Nicholas Jankowski <nrjank>
Group Member
Wed 16 Aug 2023 03:42:26 AM UTC, comment #1: 

According to Matlab corr function documentation: X and Y must not have the same number of columns only the same number of rows

:
"Input matrix, specified as an n-by-k2 matrix when X is specified as an n-by-k1 matrix."


So stepwisefit from statistics should work with the octave corr and the corr function should not throw an error "corr: X and Y must be the same size"


Anonymous
Tue 15 Aug 2023 10:48:24 AM UTC, original submission:  

stepwisefit BIST

pkg load statistics;
pkg list -->
.local/share/octave/api-v58/packages/statistics-1.6.0

X = [7 1 11 11 7 11 3 1 2 21 1 11 10; ...
     26 29 56 31 52 55 71 31 54 47 40 66 68; ...
     6 15 8 8 6 9 17 22 18 4 23 9 8; ...
     60 52 20 47 33 22 6 44 22 26 34 12 12]';
y = [78.5 74.3 104.3 87.6 95.9 109.2 102.7 72.5 93.1 115.9 83.8 113.3 109.4]';

>> [X_use, b, bint, r, rint, stats] = stepwisefit(y, X);


error: corr: X and Y must be the same size
error: called from
    corr at line 78 column 7
    stepwisefit at line 94 column 23

Likely caused by cov.m: Overhaul function for matlab compatibility (bug #50583)

Anonymous

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    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
    2024-01-12 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 9.1.0
    2023-08-17 nrjank StatusConfirmed Ready For Test
        Planned ReleaseNone 9.1.0
    2023-08-16 nrjank StatusNone Confirmed
        Operating SystemGNU/Linux Any
        Summarystepwisefit from statistics package version 1.6.0 is not working any longer with changed corr function corr produces input validation error with two inputs having different number of columns

    Back to the top

    Powered by Savane 3.13-4ef1.
    Corresponding source code