search

theislab / scib

Benchmarking analysis of data integration tools
MIT License
294 stars 63 forks source link

fix kbet metric #257

Closed mbuttner closed 3 years ago

LuckyMD commented 3 years ago

Is the observed rejection rate scaled in the same way that we expect? Would it be between 0 and 1 now?

mbuttner commented 3 years ago

Is the observed rejection rate scaled in the same way that we expect? Would it be between 0 and 1 now?

In scIB.metrics.metrics, we scale the kBET output with score = 1 - np.nanmean(kBET_score). In scIB, we scaled all metrics such that 0 means poorest and 1 means best possible. An observed rejection rate (here named kBET_score) of 0 is best and 1 is poorest. Therefore, the scaling in the scIB.metrics.metrics function is correct.

LuckyMD commented 3 years ago

Okay, cool... so the scaling would be exactly the same as for p-values then. It should probably also be somewhat correlated at least.

LuckyMD commented 3 years ago

Let me know when this is locally tested. @mumichae can you run this with your pipeline test, or is kBET not in that environment due to the R-python compatibility question.

mbuttner commented 3 years ago

Okay, cool... so the scaling would be exactly the same as for p-values then. It should probably also be somewhat correlated at least.

Well, anti-correlated, I assume. Low p-values = high observed rejection rates and vice versa.

mbuttner commented 3 years ago

Let me know when this is locally tested. @mumichae can you run this with your pipeline test, or is kBET not in that environment due to the R-python compatibility question.

Test is currently running on pancreas data - it's running, but I have to check the results.

LuckyMD commented 3 years ago

Let's keep this on a branch for now though. @danielStrobl can you create the docker image with this branch?

mumichae commented 3 years ago

Let me know when this is locally tested. @mumichae can you run this with your pipeline test, or is kBET not in that environment due to the R-python compatibility question.

There wasn't much change in the API and the pipeline runs through as expected. metrics_plot

LuckyMD commented 3 years ago

That looks like it's changed, but still quite correlated on the test data, is that correct? Could you check for correlation?

mumichae commented 3 years ago

I wouldn't interpret too much into this data, as it's just an implementation test without any representative batches

LuckyMD commented 3 years ago

Yeah, I understand... still surprising as we're expecting it to essentially invert.

LuckyMD commented 3 years ago

@mumichae could you resolve conflicts with the current master and then merge?