xhluca / bm25s

Fast lexical search implementing BM25 in Python using Numpy, Numba and Scipy
https://bm25s.github.io
MIT License
862 stars 35 forks source link

Maybe use `time.monotonic` instead of `time.time`? #42

Closed dantetemplar closed 2 months ago

dantetemplar commented 2 months ago

time.time() is commonly used to measure elapsed time, but it can be unreliable because it depends on the system clock, which can be adjusted. This can lead to incorrect time measurements.

Why time.monotonic?

time.monotonic() provides a clock that only moves forward and is unaffected by system clock changes, making it ideal for accurate time interval measurements, such as in timeouts and performance benchmarks.

Proposal

Replace time.time() with time.monotonic() in the codebase: https://github.com/xhluca/bm25s/blob/73c7dea9ea7f88a23a7fa9a94e9a7bca48669f1c/bm25s/utils/benchmark.py#L36 https://github.com/xhluca/bm25s/blob/73c7dea9ea7f88a23a7fa9a94e9a7bca48669f1c/bm25s/utils/benchmark.py#L44 https://github.com/xhluca/bm25s/blob/73c7dea9ea7f88a23a7fa9a94e9a7bca48669f1c/bm25s/utils/benchmark.py#L61 https://github.com/xhluca/bm25s/blob/73c7dea9ea7f88a23a7fa9a94e9a7bca48669f1c/bm25s/utils/benchmark.py#L76 ...ok, there are too much to list, I will make pr

xhluca commented 2 months ago

Solved in https://github.com/xhluca/bm25s/pull/44