covaruber / sommer

36 stars 23 forks source link

GWAS entries in CHANGELOG and documentation #47

Closed NeuKon closed 1 year ago

NeuKon commented 1 year ago

Hi @covaruber,

Thank you very much for your work on this amazing package.

The CHANGELOG mentions the following under ## PENDINGS AND PRIORITIES: 1) enable P3D argument

2) Enable GWAS for unimputed markers?

In addition, the GWAS documentation suggests the marker matrix M must be encoded as -1,0,1. In general, mmer and thus GWAS should be able to cope with different SNP values (e.g. 0,1,2,3,4 for tetraploids or ratio values [0, 1] for alternative alleles over total read depth), at least for additive models? Perhaps this is just an issue for some "convenience" features like MAF checks etc? In this case only small changes might be required (e.g. a warning if marker values are anything else than -1,0,1.

Best wishes, Konrad

EDIT: Rephrased the last point about marker encoding

covaruber commented 1 year ago

Konrad, thanks a lot for the feedback. I will work on the documentation. Indeed, can be misleading. Currently, the current behavior is that P3D=TRUE, of course P3D=FALSE would be estimating one marker at the time. The reason why is not enabled is more due to the way currently I am using the Rcpp code. The current Rcpp implementation for P3D=TRUE fits the emmer model, extracts the VCs and does the for loop for each marker in Rcpp. I just haven't had the time to code the for loop for P3D=FALSE. Mostly for having other priorities (the mmec() function and rebuilding mmer()) but feel free to code it and send a pull request :) otherwise I may do it in this quarter.

The coding part is a great suggestion, I should rewrite the documentation. I'll do that right away. Thanks.

Cheers, Eduardo

covaruber commented 1 year ago

P3D enabled and pushed to GitHub, to be tested now.

Cheers, Eduardo