calculateLD.R: Fixed incorrect SNP map
#178
Conversation
deepchocolate
changed the title
calculateLD.R: Fixed incorrect SNP map
…omorment/containers into deepchocolate/calculate_ld_fix
|
The "Greetings" action failure is outside our control and can be ignored: actions/first-interaction#286 |
There was a problem hiding this comment.
Thanks @deepchocolate. Looks good to me, only bumped the version file + a couple minor fixes. The LD unittests failed on my end however, because of wrong file path(s). I've pushed a couple of fixes hardcoding the /ldpred2_ref mount point in calls pointing to these datas.
Great, thanks for the review! I planned to do a test run on real data prior to merging, but haven't been able to login to TSD since Friday, so leaving it hanging here until that. |
Perhaps good to run a test in MoBA or similar. But can confirm, TSD p697 rhel login machines are unreachable. But you can use Windows VM -> Putty -> p697-appnode-norment01 to get access to the resource. |
| #' @param quantile Range of estimates to keep | ||
| getBetasAuto <- function (fitAuto, quantile=0.95, verbose=T) { | ||
| range <- sapply(fitAuto, function (auto) diff(range(auto$corr_est))) | ||
| # Keep chains that pass the filtering below |
There was a problem hiding this comment.
@espenhgn I noted that the problem om missing values start here (l199) and not at the quantile function below. There needs to be a na.rm=T to range as well. This is because if there are missing values in corr_est there will be missing in range and these will inevitably lead to missing values when assigning the keep variable below. Right now all chains with any missingness are thrown away, and I'm unsure if that's optimal
There was a problem hiding this comment.
Not sure either.
Don't think it matters here, but perhaps best not to define a variable range replacing the base::range function within this function.
There was a problem hiding this comment.
Yea, that's a good point, will change the name
There was a problem hiding this comment.
na.rm=T in range causes other issues if an entire chain is missing, so will leave for now and address later.
scripts/pgs/LDpred2/calculateLD.R
Outdated
| # Extract individuals | ||
| if (!is.na(extractIndividuals)) { | ||
| indIds <- filterFromFile(obj.bigSNP$fam, extractIndividuals, colFilter='sample.ID') | ||
| cat('Extracting ', sum(indIds), ' individuals\n') |
There was a problem hiding this comment.
This line throws an error as indIds is the same object type as obj.bigSNP$fam. sum should be changed to nrow and on line 103 indIds should have been indIds$fam$sample.ID. Will change name of indIds to something better.
This was not detected in the tests and as far as I can see this parameter does not have a test. Will add that.
|
Hi @deepchocolate; @ofrei and I discussed restructuring some of the documentation and figured it's good to merge this soon. Is it ready enough? |
Yes, I can merge it today. There's still issues with calculating your own LD as it causes optimization errors during scoring. But I will leave that for now. I've tested most other things on MoBa data now and that has worked fine. |
Fixes #177
Fixes #227
Changes proposed in this pull request:
calculateLD.Rtests/test_LDpred2/scripts/ld.shldpred2.Rusing LD data filtered by--extract(SNPs) and--sample-individualssplitLD.RanalyzeLD.R: Example plot:Plotting functionality modified from: privefl/bigsnpr#4
Before submitting
section of the
CONTRIBUTINGdocs.Writing docstrings section of the
CONTRIBUTINGdocs.