kgori / sigfit

Flexible Bayesian inference of mutational signatures
GNU General Public License v3.0
33 stars 8 forks source link

`multiplier` prior in EMu model #39

Closed baezortega closed 5 years ago

baezortega commented 5 years ago

In the model section of the sigfit_fit_emu.stan, the prior for the multiplier (real array of size G) is defined inside the for loop, but without indexing. I guess we have to add the index [i], but I don't know if this prior definition is already vectorised. (I'm just wondering if multiplier and exposures would admit vectorised prior definitions here.)

model {
    for (i in 1:G) {
        // Priors
        exposures[i] ~ dirichlet(kappa);
        multiplier ~ cauchy(0, 1);   ###  multiplier[i] ~ cauchy(0, 1); ??

        // Likelihood
        counts[i] ~ poisson(expected_counts[i]);
    }
}
kgori commented 5 years ago

That's a mistake. It should be multiplier[i] ~ Cauchy if done inside the loop, or multiplier ~ Cauchy, once, outside the loop. Probably the second is more efficient. counts and exposures are correct as they are.

On Sun, 27 Jan 2019, 17:41 Adrian Baez-Ortega <notifications@github.com wrote:

In the model section of the sigfit_fit_emu.stan https://github.com/kgori/sigfit/blob/master/exec/sigfit_fit_emu.stan, the prior for the multiplier (real array of size G) is defined inside the for loop, but without indexing. I guess we have to add the index [i], but I don't know if this prior definition is already vectorised. (I'm just wondering if multiplier and exposures would admit vectorised prior definitions here.)

model { for (i in 1:G) { // Priors exposures[i] ~ dirichlet(kappa); multiplier ~ cauchy(0, 1); ### multiplier[i] ~ cauchy(0, 1); ??

    // Likelihood
    counts[i] ~ poisson(expected_counts[i]);
}

}

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/kgori/sigfit/issues/39, or mute the thread https://github.com/notifications/unsubscribe-auth/ABkM_6UTn0mbyb72ev98EYDViFhGeu9Fks5vHeTjgaJpZM4aU140 .

baezortega commented 5 years ago

That's what I thought. I realised exposures are already vectorised! I'll fix this in dev. On 27 Jan 2019 19:08, "Kevin Gori" notifications@github.com wrote:

That's a mistake. It should be multiplier[i] ~ Cauchy if done inside the loop, or multiplier ~ Cauchy, once, outside the loop. Probably the second is more efficient. counts and exposures are correct as they are.

On Sun, 27 Jan 2019, 17:41 Adrian Baez-Ortega <notifications@github.com wrote:

In the model section of the sigfit_fit_emu.stan https://github.com/kgori/sigfit/blob/master/exec/sigfit_fit_emu.stan, the prior for the multiplier (real array of size G) is defined inside the for loop, but without indexing. I guess we have to add the index [i], but I don't know if this prior definition is already vectorised. (I'm just wondering if multiplier and exposures would admit vectorised prior definitions here.)

model { for (i in 1:G) { // Priors exposures[i] ~ dirichlet(kappa); multiplier ~ cauchy(0, 1); ### multiplier[i] ~ cauchy(0, 1); ??

// Likelihood counts[i] ~ poisson(expected_counts[i]); } }

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/kgori/sigfit/issues/39, or mute the thread https://github.com/notifications/unsubscribe-auth/ABkM_ 6UTn0mbyb72ev98EYDViFhGeu9Fks5vHeTjgaJpZM4aU140 .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kgori/sigfit/issues/39#issuecomment-457945007, or mute the thread https://github.com/notifications/unsubscribe-auth/AJuInIjo6VeYbxerE6U8LHf7412ujCjzks5vHfk6gaJpZM4aU140 .