8000 clarification on OAS estimator formula (sklearn.covariance.OAS) (mean instead of trace is used) · Issue #23280 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

clarification on OAS estimator formula (sklearn.covariance.OAS) (mean instead of trace is used) #23280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
assuntaciarlo opened this issue May 4, 2022 Discussed in #23260 · 1 comment · Fixed by #23867
Closed

clarification on OAS estimator formula (sklearn.covariance.OAS) (mean instead of trace is used) #23280

assuntaciarlo opened this issue May 4, 2022 Discussed in #23260 · 1 comment · Fixed by #23867
Labels
module:covariance Needs Investigation Issue requires investigation

Comments

@assuntaciarlo
Copy link

Dear sklearn experts,

I was comparing different shrinkage algorithms and when looking at sklearn implementation of the OAS estimator I found something strange in the definition of the shrinkage factor or at least not clear to me. In the original formula from Chen et al. 2010 (the formula is also wrong in the paper, anyway) they used always the trace of the covariance matrix. Instead, this is the formula from sklearn.covariance.OAS module:

 mu = np.trace(emp_cov) / n_features
# formula from Chen et al.'s **implementation**
alpha = np.mean(emp_cov ** 2)
num = alpha + mu ** 2
den = (n_samples + 1.) * (alpha - (mu ** 2) / n_features)

shrinkage = 1. if den == 0 else min(num / den, 1.)
shrunk_cov = (1. - shrinkage) * emp_cov
shrunk_cov.flat[::n_features + 1] += shrinkage * mu

where alpha is the mean of the squared covariance matrix instead of the trace, and also the mu parameter is normalized by the number of features also at the numerator, differently from what I found in the literature, referring to the same formula.
A trascription of what I found in papers should be (discarding the factor 2/p as in sklearn):

 mu = np.trace(emp_cov) 
alpha = np.trace(emp_cov ** 2)
num = alpha + mu ** 2
den = (n_samples + 1.) * (alpha - (mu ** 2) / n_features)

shrinkage = 1. if den == 0 else min(num / den, 1.)
shrunk_cov = (1. - shrinkage) * emp_cov + shrinkage * np.diag(np.diag(emp_cov))

Is this right? Are the two forms equivalent in a way that I couldn't understand?

Thank you in advance,
Assunta
@github-actions github-actions bot added the Needs Triage Issue requires triage label May 4, 2022
@Micky774 Micky774 added module:covariance Needs Investigation Issue requires investigation and removed Needs Triage Issue requires triage labels Jul 8, 2022
@Micky774 Micky774 mentioned this issue Jul 8, 2022
Merged
@Micky774
Copy link
Contributor
Micky774 commented Jul 8, 2022 •
edited
Loading

Hey there @assuntaciarlo! Sorry for the long delay with this issue. After reading into original publication (I believe it's been corrected) and considering the R implementation I'm inclined to agree with you that the current implementation is off. In particular, the $\mu^2$ terms in the numerator and denominator resolve to $\operatorname{tr} (\hat S)^2 / M^2$. Should be a fairly quick fix. Opened a PR here: #23867

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
module:covariance Needs Investigation Issue requires investigation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

MAINT add more intuition on OAS computation based on literature Micky774/scikit-learn
2 participants
@Micky774 @assuntaciarlo
0