num-complex icon indicating copy to clipboard operation
num-complex copied to clipboard

more accurate sqrt function

Open pascalkuthe opened this issue 1 year ago • 9 comments

I noticed that the square root implementation in num-complex uses conversion trough polar coordinates to compute complex squre roots. Usually the algorithm from https://dl.acm.org/doi/abs/10.1145/363717.363780 is used to compute the complex square root instead.

The algorithm uses hypot/norm and square root to compute csqrt. This approach should be faster since less transcendental calls are needed. Hypot and sqrt also tend to be faster compared to exp/ln/atan/cos/sin. Both hypot and sqart also have much higher precision. Most implementations guarantee that these two functions return the correctly rounded infinite accuracy result.

For prior art you can look at the glibc and musl implementation (https://git.musl-libc.org/cgit/musl/tree/src/complex/csqrt.c). The glibc implementation is a lot more complicated/hard to read because they ensure that underflow floating point exceptions are triggered correctly. I don't think that is something num-complex needs to do. There is also some accuracy loss for subnormal numbers that I didn't handle yet. I left a comment about it, but it is very minor.

pascalkuthe avatar May 11 '24 11:05 pascalkuthe

Thanks for the fast review! Apologies for not getting back to this yet.

I had to unexpectetly travel for work so I will not be able to get back to this before the weekend.

pascalkuthe avatar May 21 '24 06:05 pascalkuthe

This should be ready for another round of review, apologies again for the delay

pascalkuthe avatar May 28 '24 12:05 pascalkuthe

@cuviper still quite interested in this.

Anything I can do to push this over the finish line?

pascalkuthe avatar Dec 08 '24 20:12 pascalkuthe

How about tests that demonstrate the improved accuracy?

cuviper avatar Dec 11 '24 00:12 cuviper

@cuviper testing for accuracy is tricky. I used numpy (which generates the same results as glibc/musl) to generate some reference numbers for some cases that round differently with the current implementation (but the same with the new one).

This is a bit of a chicken and egg problem with showing that the rounding is truely more accurate but since this is a well known algorithm (and very well known implementations) I think it's a fair approach.

I choose some arbitrary examples to demonstrate accuracy. The current implementation rounds incorrectly for a ton of numbers so it's not practical to test exhaustively

pascalkuthe avatar Dec 14 '24 23:12 pascalkuthe

Does the accuracy show itself by comparing the result squared to the original value? You may be able to use simple inputs without having to hard-code expected results. Hopefully that round trip is better now than it was before, at least for some inputs. (and hopefully not any worse in general, but I agree we can't really be exhaustive about it)

cuviper avatar Dec 14 '24 23:12 cuviper

I did find a few where the roundtrip was better by an ULP or two. Some of the accuracy gets lost (particularly since it's multiple operations for complex) during the multiplication so its not perfect test (makes it a bit harder to find cases) but a good addition, thanks!

During some trial and error I also never had any case where accuracy was worse.

It was quite a while since I worked on this but I think I also semi exhaustively (somewhat sampled) tested against glibc at some point IIRC. With this algorithm I am pretty certain that existing implementations get this right so perfectly matching those is the test that gave me the most confidence in the implementation

pascalkuthe avatar Dec 15 '24 01:12 pascalkuthe

Is there anything I can help out in to get this integrated?

PaulXiCao avatar Oct 25 '25 18:10 PaulXiCao

From my side this js done, just waiting on a review from @cuviper

pascalkuthe avatar Oct 26 '25 18:10 pascalkuthe