sgkit icon indicating copy to clipboard operation
sgkit copied to clipboard

VanRaden GRM

Open timothymillar opened this issue 1 year ago • 1 comments

Fixes #874

I've tried to preemptively make this function suitable for additional estimators if/when required.

One point I'm unsure on is if we should have a default for ancestral_frequency based on the sample population. It's common practice to use the sample population allele frequency as an estimate of the ancestral allele frequency although this is often inappropriate. So the question is whether it's best to force an explicit choice by the user, or default to the (potentially inappropriate) most common choice?

timothymillar avatar Sep 14 '22 04:09 timothymillar

So the question is whether it's best to force an explicit choice by the user, or default to the (potentially inappropriate) most common choice?

I think it's best to force a choice in the first instance if there's any doubt. Otherwise you can get into nasty problems of backwards compabibility, if the default that you initially chose turns out to be not quite the thing you should have done.

jeromekelleher avatar Sep 14 '22 10:09 jeromekelleher

Is this ready to be merged or are you planning further changes @timothymillar?

tomwhite avatar Sep 28 '22 16:09 tomwhite

I think it's good to go, merging

timothymillar avatar Sep 29 '22 00:09 timothymillar