stephenslab / ebnm

R package to fit Empirical Bayes Normal Means model.
https://stephenslab.github.io/ebnm/
12 stars 9 forks source link

plan to produce uniform interface for more G choices #16

Closed stephens999 closed 5 years ago

stephens999 commented 5 years ago

The proposal is to add a bunch of functions (mostly wrappers to ashr) to implement a variety of ebnm functions. The goal is to provide, as much as possible, a uniform interface: ie as much as possible the inputs and outputs will be the same format/names.

Possible function names: ebnm_normal_scale_mixture ebnm_unimodal_nonnegative ebnm_unimodal_nonpositive ebnm_unimodal_symmetric ebnm_unimodal ebnm_ash (a general interface to ash, which just passes things to ash but produces output in the same format as other ebnm functions). ebnm_npmle

stephens999 commented 5 years ago

@willwerscheid suggests using parameters mode and mixsd (or sd?) to specify mode and standard deviations will allow interface to look uniform.

Possibly: ebnm_normal(x, s, mode=0, sd = 1) ebnm_normal(x, s, mode="estimate", sd = "estimate") ebnm_normal(x, s, mode="estimate", sd = 1) ebnm_normal(x, s, mode=2, sd = "estimate")

ebnm_point_normal(x, s, mode=0, sd = 1) ebnm_point_normal(x, s, mode=0, sd = "estimate") ebnm_point_normal(x, s, mode="estimate", sd = 1)

ebnm_unimodal(x, s, mode="estimate", sd = "autoselect")
ebnm_unimodal_symmetric(x, s, mode="estimate", sd = "autoselect")

Some thoughts:

i) if we estimate mode, ebnm_unimodal_nonnegative is a misnomer. We could avoid this by ebnm_unimodal_righttail but maybe we are fine with the misnomer given this is likely by far the most common usage. In ash we use uniform+. Is ebnm_unimodal+ a better name than ebnm_unimodal_nonnegative?

ii) for uniform mixtures, mixsd in ash is actually used for the limits not the standard deviations. so mixsd = c(0.1,0.2), mixcompdist="uniform" gives a mix of U(-0.1,0.1) and U(-0.2,0.2)
mixsd = c(0.1,0.2), mixcompdist="halfuniform" gives a mix of U(0,0.1), U(0,0.2), U(-0.1,0), U(-0.2,0)

I guess technically for halfnormal(truncated normals) the sds are also technically not the sds of the mixture components (which are truncated normal), but the sds of the normal before truncation.

To keep consistency/simplicity we should probably use the same convention?

iii) If we have a g_init parameter for warmstart then we have to decide what consistency to enforce.

My guess is that g_init will usually be from a previous fit, so all the parameters in g_init will be specified.

So if we have ebnm_point_normal(x,s, mode=0, sd=1, g_init = get_fitted_g(previous.fit) we will check that g_init is consistent with any numeric values of mode and sd provided and throw error if not.

If we have ebnm_ash(x,s, mode=0, sd="autoselect", g_init = get_fitted_g(previous.fit) then we have to decide whether to throw an error or not (since the g_init provides sd, so it isn't possible to both autoselect and use g_init).

willwerscheid commented 5 years ago

ebnm_unimodal+ is not a valid function name.

stephens999 commented 5 years ago

ok then ebnm_unimodal_nonnegative ?

willwerscheid commented 5 years ago

Most of the above suggestions have been implemented; ebnm_npmle remains.

willwerscheid commented 5 years ago

I will close since the primary goal of the issue has been accomplished. I'll create ebnm_npmle as a separate issue (enhancement).