-
Notifications
You must be signed in to change notification settings - Fork 0
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
statistics: q.VgB should be np/(np-1) q.VgB #27
Comments
Hi @jgx65 , I wanted to have a quick look at this issue before shipping the new version to see if we could include a fix. It is likely that we could just change this line: Line 3640 in 331c01e
But then the question is how many times do we do the same mistake. I guess at least we have the same problem 10 lines above?! Line 3630 in 331c01e
And the function var is also used in function to compute stat within patch: Line 3140 in 331c01e
(this happen 7 times for various similar quantities. Just search for var( )
Finnaly, we have two cases (not sure what we are computing here, but this is probably more clear to you) where we use it for an array of size 2: Line 3523 in 331c01e
Here, we have a factor 2 in the variance if I'm not mistaken. In principle, we should check all these statistics, see if we observe the systematic bias, design test to check it, and then update the code to see if the bias disapear. I don't really know how easy/difficult this would be and who would do it. How do you want to move forward with this? Cheers, |
@frederic-michaud I had a look, I believe in all cases the n/(n-1) factor should be included. And you are right, in the last case, it is a factor of 2 since n=2! The only situation where the n/(n-1) multiplying factor should not be used is if the mean is derived from an independent set of data, but here, I don't think it is the case. @sneuensc, would you agree? |
Hi Jerome,
That is fine for me. Personally I don’t mind which statistic is computed, but it has to be stated in the manual.
Cheers,
Sam
… On 10 Jun 2020, at 10:46, Jerome Goudet ***@***.***> wrote:
@frederic-michaud <https://github.com/frederic-michaud> I had a look, I believe in all cases the n/(n-1) factor should be included. And you are right, in the last case, it is a factor of 2 since n=2! The only situation where the n/(n-1) multiplying factor should not be used is if the mean is derived from an independent set of data, but here, I don't think it is the ***@***.*** <https://github.com/sneuensc>, would you agree?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#27 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACY3ECLV2I7CKKDBDLPKNBDRV5B7BANCNFSM4M7VTQPA>.
|
Ok, so I update the code and push? Will you have time to quickly check some statistics? |
thanks @frederic-michaud, I'll try to do some checking. |
I checked the manual, we always say 'variance' without specifying which formula we are using |
Thank you for looking gat it. Would it make sense to write somewhere globally (at the start of the set section) that the stats are estimates, respectively corrected for sample size?
Cheers,
Sam
… On 10 Jun 2020, at 12:01, Jerome Goudet ***@***.***> wrote:
Hi Jerome, That is fine for me. Personally I don’t mind which statistic is computed, but it has to be stated in the manual. Cheers, Sam
… <x-msg://5/#>
I checked the manual, we always say 'variance' without specifying which formula we are using
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#27 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACY3ECIQR43XSFT5UC3NZX3RV5KV3ANCNFSM4M7VTQPA>.
|
Thanks @frederic-michaud . It seems to work from the little checking I have made, results are in lines with what I expect. |
The between patch genetic variance
q.VgB
necessary to estimateQST
is underestimated by a factornp/(np-1)
, wherenp
is the number of patches.The text was updated successfully, but these errors were encountered: