bugGNU Octave - Bugs: bug #48315, cov.m: Incorrect result

 
 

bug #48315: cov.m: Incorrect result

Submitter:  Ryan Thompson <locu4096>
Submitted:  Sat 25 Jun 2016 12:32:56 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  lachlan
Originator Name:  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

Mon 08 Aug 2016 11:05:53 PM UTC, comment #7: 

Yes, that's why I added the note to NEWS.  I think most people, however, will never even notice.  First, they are likely to be using real-valued matrices if they are doing statistics on the real world.  Second, even if they are using complex numbers, they are probably interested in the magnitude of the correllation and are using a call to abs() somewhere in the code.  I think we would have got a lot more complaining users earlier if they had been depending on the phase of the complex value returned.

Rik <rik5>
Group administrator
Mon 08 Aug 2016 10:49:57 PM UTC, comment #6: 

If I understand correctly, code expecting the previous behavior will now silently return a different answer.  Is that correct?

John W. Eaton <jwe>
Group administrator
Mon 08 Aug 2016 10:42:45 PM UTC, comment #5: 

@Lachlan: I pushed your cset here http://hg.savannah.gnu.org/hgweb/octave/rev/9a0e30e24b9b.  I added a note about the change to the NEWS file since this could conceivably bite some people, although most people are probably calculating real-valued covariances.

Fixed, closing report.

Rik <rik5>
Group administrator
Fri 01 Jul 2016 09:53:38 AM UTC, comment #4: 

Please use file #37640 instead -- it has the correct user name.

Lachlan Andrew <lachlan>
Mon 27 Jun 2016 06:09:50 AM UTC, comment #3: 

Ignore the third to second-last paragraphs of my previous comment...

I also spent a while re-implementing Matlab's behaviour for cov(A,B) before noticing the compatibility note.  Matlab's actual behaviour is much more sensible than the behaviour described in the (old) compatibility note.

If the decision to deviate from Matlab's behaviour was based on the misunderstanding in the old note, then I think the decision should be reconsidered.

The attached changeset fixes the conj issue, and fixes the compatibility note.

(file #37594)

Lachlan Andrew <lachlan>
Mon 27 Jun 2016 03:11:56 AM UTC, comment #2: 

I can confirm the Matlab output (in R213b).

If our aim is just to be Matlab compatible, then yes, I think we should just remove the conj, since that would match Matlab's definition, given at <http://au.mathworks.com/help/matlab/ref/cov.html>.

If we want to work out what the "normal" convention is, it becomes complicated.

Wikipedia defines the variance of a random vector as


 var(z) = E[(z-mu)(z-mu)*]


(note that the conjugate is on the second factor, not the first like Matlab's).

However, that assumes vectors are column vectors.  Matlab's function assumes each row is an instance of the random vector.  Is that row the transpose of z, or the conjugate transpose of z?  I haven't thought it through, but I assume that those two will give covariances that are conjugates of each other.

In short, I think the input to this function is sufficiently non-standard that we can just follow Matlab.

Lachlan Andrew <lachlan>
Sat 25 Jun 2016 08:25:41 PM UTC, comment #1: 

Thanks for the bug report. I see the same output as you in Octave. So the Octave result seems to be the conjugate of what Matlab returns.

It looks like the cov.m function in Octave has been defined as


conj (x' * y / (n-1))


since 1997.

Should the conj() just be removed?

Mike Miller <mtmiller>
Group Member
Sat 25 Jun 2016 12:32:56 PM UTC, original submission:  

Hi,

I'm getting different results when calculating the covariance of a complex matrix between Octave and MATLAB:

x = [
   1.0000 - 1.0000i  -1.0000 + 1.0000i   1.0000 - 1.0000i  -1.0000 + 1.0000i
  -1.0000 - 1.0000i   1.0000 - 1.0000i  -1.0000 + 1.0000i  -1.0000 + 1.0000i
   1.0000 + 1.0000i   1.0000 - 1.0000i  -1.0000 - 1.0000i  -1.0000 + 1.0000i
  -1.0000 - 1.0000i  -1.0000 - 1.0000i  -1.0000 + 1.0000i  -1.0000 + 1.0000i]

R = cov(x')


x = [
   1.0000 - 1.0000i  -1.0000 + 1.0000i   1.0000 - 1.0000i  -1.0000 + 1.0000i
  -1.0000 - 1.0000i   1.0000 - 1.0000i  -1.0000 + 1.0000i  -1.0000 + 1.0000i
   1.0000 + 1.0000i   1.0000 - 1.0000i  -1.0000 - 1.0000i  -1.0000 + 1.0000i
  -1.0000 - 1.0000i  -1.0000 - 1.0000i  -1.0000 + 1.0000i  -1.0000 + 1.0000i]

R = cov(x')

% % Octave 4.0.2
%
% R =
%
%    2.66667 - 0.00000i  -0.66667 - 0.66667i   0.00000 - 0.00000i   0.00000 - 0.00000i
%   -0.66667 + 0.66667i   2.33333 - 0.00000i   0.66667 + 0.66667i   1.33333 - 0.66667i
%    0.00000 + 0.00000i   0.66667 - 0.66667i   2.66667 - 0.00000i   0.00000 - 1.33333i
%    0.00000 + 0.00000i   1.33333 + 0.66667i   0.00000 + 1.33333i   1.33333 - 0.00000i
%
% % MATLAB 2014A
%   
% R =
%
%    2.6667 + 0.0000i  -0.6667 + 0.6667i   0.0000 + 0.0000i   0.0000 + 0.0000i
%   -0.6667 - 0.6667i   2.3333 + 0.0000i   0.6667 - 0.6667i   1.3333 + 0.6667i
%    0.0000 + 0.0000i   0.6667 + 0.6667i   2.6667 + 0.0000i   0.0000 + 1.3333i
%    0.0000 + 0.0000i   1.3333 - 0.6667i   0.0000 - 1.3333i   1.3333 + 0.0000i

Ryan Thompson <locu4096>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #37640:  bug_48315_cov_conj.cset added by lachlan (2KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by locu4096 (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-08-08 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2016-07-01 lachlan Attached File#37594 Removed
    2016-07-01 lachlan Attached File- Added bug_48315_cov_conj.cset, #37640
    2016-06-27 lachlan Attached File- Added bug_48315_cov_conj.cset, #37594
        StatusConfirmed Patch Submitted
        Assigned toNone lachlan
        Release4.0.2 dev
    2016-06-27 lachlan StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code