stephens999 / ashr

An R package for adaptive shrinkage
GNU General Public License v3.0
80 stars 36 forks source link

never drop mixture components for which prior > 1 #122

Closed willwerscheid closed 4 years ago

willwerscheid commented 4 years ago

We've been dropping columns from the likelihood matrix that are identically zero. This causes the corresponding mixture proportion to be estimated as zero. This is the right thing to do when the prior parameter for that mixture component is 1, but it's otherwise incorrect. I've changed to only drop when the likelihood is identically zero AND the prior parameter is 1. This works fine with mixsqp, but I'm not familiar enough with the other optimization methods to be confident that the change won't break anything. Would you mind taking a look @pcarbo ?

pcarbo commented 4 years ago

@willwerscheid The estimate_mixprop code is in desperate need of refactoring—it is unnecessarily complicated and confusing. (I'm not suggesting you do the refactoring. Just pointing this out.)

Your change looks good, except that prior is a vector, so your check should look like any(prior > 1), right?

willwerscheid commented 4 years ago

No -- we still need to drop columns where the likelihood is identically zero and prior == 1 since zero columns cause mixsqp to fail (or at least they used to do). When prior > 1, then observations essentially get added to the likelihood matrix which means that columns are no longer zero for those components.

pcarbo commented 4 years ago

Okay, nevermind. I had trouble parsing the parentheses.