TileDB
TileDB copied to clipboard
Pairwise summation floats
This adds a pairwise summation function for floating point types.
TYPE: FEATURE DESC: Pairwise summation floats
This pull request has been linked to Shortcut Story #33201: Implement pairwise summation for floats.
It would be better if this function were extensible to allow integral types. This is an interface issue, not a request for such code in the current PR. One goal would be to remove any explicit type dependency (such as
log2fvs.log2l) from the unit tests and into a template of some sort.
@eric-hughes-tiledb I don't really understand why I would extend this to integral types, I didn't have much of a requirements list for this function, but I definitely saw that it's a floating point summation function, please let me know if I'm missing something.
Regarding type dependency, log2f and log2l I think can go away with c++23 when log2 gets added.
Overflow would need to be dealt with. You'd also need a new equality operation (not
operator==) for testing; it would admit the difference between exactly equal and approximately equal.
Do we have plans to turn this into a floating point operations library of some sorts? This function was pitched to me like "1-2 hours of work", thus I added a simple pairwise summation implementation + a test.
@eric-hughes-tiledb updated the code based on what we last discussed. I have some comments/questions here:
- I adjusted generated vectors from 2^24 as we discussed to 2^22 as the test was quite slow.
- It's the first time I use catch2 benchmark, so I just put the small snippets in the unit test. Where are these benchmarks supposed to be placed? Do we want them to be run in the CI given they are quite slow?
- static operator() overloads need to wait till c++23