fwhelan / coinfinder

A tool for the identification of coincident (associating and dissociating) genes in pangenomes.
GNU General Public License v3.0
95 stars 10 forks source link

Question about significance correction #45

Closed chg60 closed 3 years ago

chg60 commented 3 years ago

In coinfinder/coinfind-code/coincidence.cpp (line 42) there is this line of code:

const double cor_sig = Significance::correct( dataset.get_options().sig_level, dataset.get_options().correction, (((size_alpha_table)*(size_alpha_table+1))/2));

Where n_tests is being calculated as (n_alphas (n_alphas + 1)) / 2. Should it not be (n_alphas (n_alphas - 1)) / 2? These are pairwise gene-gene tests, so the most tests there can possibly be is n_alphas * n_alphas, right?

fwhelan commented 3 years ago

Hi chg60,

Yes! I think you're right! What a good spot- thank you so much for bringing this to my attention. I'm going to take an hour or so to really think about this and go through my notes to see if I have a justification for why its coded this way; if not, I will fix this, include a bug report, and release a new version right away (and update you here).

Thank you so much for posting this issue (and apologies for not getting back to you sooner!).

fwhelan commented 3 years ago

pushed commit 0335628 with fix

fwhelan commented 3 years ago

pushed a new release https://github.com/fwhelan/coinfinder/releases/tag/1.0.9

this should be propagated to bioconda momentarily, but I'll keep an eye on it...

chg60 commented 3 years ago

Very cool, thank you! I'll try pulling the new conda package shortly.

No worries about the delay - this was actually much quicker than many of the other packages I've posted minor bug reports on!