ip200 / venn-abers

Python implementation of binary and multi-class Venn-ABERS calibration
MIT License
114 stars 11 forks source link

Performance comparison #4

Closed karllandheer closed 11 months ago

karllandheer commented 11 months ago

Hello, I would like to start off by saying thank you for this great package, and your great work with CPs. This is some of the most important work being done in the ML space today. I have a question/concern about the performance benchmarking. I compared your implementation to the https://github.com/ptocca/VennABERS implementation. I tried with a dataset of 50,000 calibration/test points, and the ptocca method took 0.07 seconds, whereas your implementation took 104 seconds. These weren't random values but actual data. I tried with my full calibration/test set (several hundred k points), and the ptocca implementation finished after 30 seconds, while I quit the new one after ~20 mins or so. I think this is in line with what your results were showing: for small/medium datasets your implementation is faster, but as the dataset gets large the ptocca implementation becomes better. It is perhaps worth noting my dataset is very skewed (almost all points are near 0, with a few at values near 1, and nearly none in-between)

ip200 commented 11 months ago

Hi, thank you so much for your message and for flagging this issue. I have just run a comparison with 500,000 random calibration and test points and it appears that the ptocca method is still slower. But as you say this may be because I am testing using a randomly generated dataset - I will try and replicate this using skewed values and see what's causing this. Will be in touch soon! Thanks, Ivan

ip200 commented 11 months ago

Hi, I have created a new branch https://github.com/ip200/venn-abers/tree/floating_point which I hope will alleviate the slow running time you experience with very large datasets. If you have time to try, please let me know whether this resolves the issue. Thanks, Ivan

karllandheer commented 11 months ago

Hello, thank you for your prompt reply. I will check this on Monday and get back to you

karllandheer commented 11 months ago

Hello, sorry I may be making a mistake but the comparison I was recreating is in this notebook:

https://github.com/ip200/venn-abers/blob/floating_point/examples/comparison_with_existing.ipynb

specifically, using your function VennABERS_by_def. I do not think your new branch changed VennABERS_by_def, hence the performance will be identical. Am I missing something?

ip200 commented 11 months ago

Hi, thank you and apologies for any confusion. The function VennABERS_by_def is actually a function which calculates Venn ABERS scores and probabilities manually and is not a part of the Venn-ABERS package. It is mainly used here to compare the performance of my method and the method of ptocca with the manual method VennABERS_by_def, which is exact but very slow by design. So the running time comparison between this package and the existing one can be found in cells 8 and 5 respectively within https://github.com/ip200/venn-abers/examples/comparison_with_existing.ipynb (i.e ignoring cell 6 which is the manual very slow method). The branch https://github.com/ip200/venn-abers/tree/floating_point contains a small modification to the calc_p0_p1 function which makes it run much faster for large imbalanced datasets while not losing much in terms of accuracy of calibration. If you find this modification useful, I can easily incorporate it as a further optional parameter within the main VennAbersCalibrator class.

karllandheer commented 11 months ago

Hello, oh my mistake. OK yes I have compared the three implementations with 500k calibration points, 16777216 test points. The comparison results are:

ptocca method: 6.0 s ip200 method v1: 0.55 s ip200 method v2: 0.21 s

Perhaps it would be good to include this as a flag? I don't know how accuracy is lost by your v2, but the speed up for my dataset is large. Thank you very much for all your help!

I have another unrelated issue but I will open another thread

ip200 commented 11 months ago

Hi, thank you - really glad to hear it helps. I'll include this as an additional optional parameter and will submit and merge with the main branch first thing tomorrow.

I'll also start looking into the other issue and will come back to you shortly.

ip200 commented 11 months ago

Hi, I have added the optional precision parameter within VennAbersCalibrator, VennAbersMultiClass and VennAbersCV as well as within the VennAbers.fit() function. An example of its usage can be found in examples/simple_classification.ipynb cells 22-23. I hope this helps, I will close the issue for now. Thank you