annahutch / corrcoverage

Repo for corrcoverage package
Other
6 stars 3 forks source link

corrected_credset.R #13

Closed chr1swallace closed 5 years ago

chr1swallace commented 5 years ago

This file contains the lines

  pp_ERR = function(Zj){
    exp.zm = Zj %*% Sigma
    mexp.zm = matrix(exp.zm, nrep, length(Zj), byrow=TRUE) # matrix of Zj replicated in each row
    zstar = mexp.zm+ERR
    bf = 0.5 * (log(1 - r) + (r * zstar^2))
    denom = logsum(bf)
    pp.tmp = exp(bf - denom)  # convert back from log scale
    pp.tmp / rowSums(pp.tmp)
  }

In this, I think zstar, and therefore bf are matrices. But denom is a scalar. I think you need

denom=apply(bf,1,logsum)
annahutch commented 5 years ago

Yes - you are correct, thanks for spotting this! Doesn't seem to change the correction much but I am re-running simulations to check.

chr1swallace commented 5 years ago

Ok. Is this code used anywhere else in your package? Seems a standard bit of code, should be a function in its own right, that is then called by different functions

http://chr1swallace.github.io

On 25 Jul 2019, 09:20, at 09:20, Anna Hutchinson notifications@github.com wrote:

Yes - you are correct, thanks for spotting this! Doesn't seem to change the correction much but I am re-running simulations to check.

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/annahutch/corrcoverage/issues/13#issuecomment-514947205

annahutch commented 5 years ago

Yep sounds good, it is used throughout so will amend now

chr1swallace commented 5 years ago

Great to organise code like that.

But I see it won't actually make a difference because this line normalises

pp.tmp/rowSums(pp.tmp)

But if you get the denom right you won't need to normalise

http://chr1swallace.github.io

On 25 Jul 2019, 09:24, at 09:24, Anna Hutchinson notifications@github.com wrote:

Yep sounds good, it is used throughout so will amend now

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/annahutch/corrcoverage/issues/13#issuecomment-514948405

annahutch commented 5 years ago

Ah yes, so I no longer need the final pp.tmp/rowSums(pp.tmp) line