SnpArrays.jl icon indicating copy to clipboard operation
SnpArrays.jl copied to clipboard

Fix a minor error with standardization of SNPs when copying

Open dohyunkim116 opened this issue 1 year ago • 2 comments

Centering and scaling were not being performed when copying SnpLinAlg into a numeric matrix with mean imputation.

dohyunkim116 avatar Feb 24 '24 01:02 dohyunkim116

I'm not sure about the point of this PR. Could you explain more about it? The original code imputes the missing value with the column mean, and the proposed code imputes the missing value with zero when s.impute is true. See the output from the unit tests.

kose-y avatar Feb 25 '24 20:02 kose-y

I think the modified code does impute with column mean, and return zero when s.center is true. The original code returned column mean regardless s.center was set to true. Mean imputation should always return zero after centering, right?

In the below test, (lines 288, 299 and 300 in runtests.jl),

mousela = SnpLinAlg{t}(mouse, model=ADDITIVE_MODEL, center=true, scale=true, impute=true)
 v = copyto!(zeros(1), @view(mousela[702])) # missing entry
 @test isapprox(v[1], 1.113003134727478, atol=1e-6)

we are checking whether the mean imputation is performed and is approximately equal to 1.113. The test should actually check whether v is equal to zero since center = true. If we have set center = false, then the test in line 300 should pass. Please correct me if I am missing something here.

dohyunkim116 avatar Feb 25 '24 21:02 dohyunkim116