Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified files for strain-specific residual variance option #269

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

jb621-star
Copy link

These changes include the modifications to the math inside mvlmm, and attempts to add an option for the user to input their own strain-specific residual variance column within the files gemma_io.cpp and param.cpp. Thanks for your help!

Jameson Blount and others added 20 commits March 20, 2023 09:52
Small formatting change to -residvar
Updated resid_var math to CalcQi
Updated most functions except MphEM and remaining P-val functions
Continued to make appropriate math changes to MphCalcSigma, UpdateV, MphCalcLogL, MphEM, MphCalcP, MphCalcBeta, and other functions.
Finished theoretical changes in other functions and replaced all "eps_eval" entries with "eval_vec, sigmasq".
Replaced any lingering mentions of "eps_eval" with "eval_vec, sigmasq".
Changed mentions of "eps_eval" with "sigmasq". Still need to figure out how to get eigenvectors from the same place "eval" is derived.
Changed mention of eps_eval as a vector to "gsl_matrix *sigmasq"
Changed the one mention of "eps_eval" to "eval_vec" and "sigmasq". May actually need to return to this file in order for the code to actually run.
eps_eval --> eval_vec, sigmasq
Changed eps_eval to sigmasq and corrected any identified regions where dimensions did not match.
Replaced input of CopyResid from eps_eval to sigmasq
Copy link

@aartaka aartaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I've noticed. Most of the errors you're getting are due to mismatch between the arguments function requires and the arguments you provide, so ensure these are in sync. The rest I've tried to locate and comment on. Let's have a call (coordinate via Matrix) in case there's something I missed.

src/param.h Outdated
@@ -136,6 +136,7 @@ class PARAM {
string file_anno; // Optional.
string file_gxe; // Optional.
string file_cvt; // Optional.
string file_residvar; // Optional
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice you called it file_residvar here, but are using it as file_resid elsewhere. A typo?

src/gemma.cpp Show resolved Hide resolved
src/gemma.cpp Outdated
@@ -2795,22 +2816,22 @@ void GEMMA::BatchRun(PARAM &cPar) {
if (!cPar.file_bfile.empty()) {
// PLINK analysis
if (cPar.file_gxe.empty()) {
cLmm.AnalyzePlink(U, eval, UtW, &UtY_col.vector, W,
cLmm.AnalyzePlink(U, eval, eval_vec, sigmasq, UtW, &UtY_col.vector, W,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't define eval_vec anywhere earlier, that's why compiler is shouting. What is the intended use for eval_vec?

Also note that sigmasq is not defined either.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then there's also required/provided argument mismatch. AnalyzePlink requires (U, eval, UtW, Uty, W, y, gwasnps), while you provide (U, eval, eval_vec, sigmasq, UtW, Uty, W, y, gwasnps). I guess AnalyzePlink needs to be modified to accept new arguments first.

src/mvlmm.h Outdated
@@ -35,6 +35,7 @@ class MVLMM {

string file_bfile;
string file_geno;
//string file_residvar maybe?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if you need to use it (although you seems to only use residuals as sigmasq, so maybe no need for it.) And don't forget to add a line assigning it to CopyFromParam in case you add this field.

Changed "file_residvar" back to "file_resid"
Transferred all instances of "eval_vec" to "U".
Changed all instances of "_residvar" to "resid" to keep things consistent.
Changed all instances of "residvar" to "resid" to keep things consistent
Changed inputs to "CalcLmmVgVeBeta", "CalcLambda", and "AnalyzePlink" according to their use elsewhere (i.e. no more "eval_vec").
Changed residvar to resid for consistency.
Changed all mentions of residvar to resid for consistency
Changed all residvar to resid for consistency.
Changed "residvar" to "resid" and removed unnecessary mentions of "eval_vec" from other functions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants