-
-
Notifications
You must be signed in to change notification settings - Fork 11.9k
ENH: Use high accuracy SVML for double precision umath functions #24006
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Benchmark numbers: On average the double precision high accuracy version seems 1.23x slower than the LA versions (results do not include sin, cos and tanh): |
Wow, I thought it would be much more of a difference, although I will note that those benchmarks seem to run in 10-20μs and may have ~1μs overhead, so the slowdown should be a bit more in practice. TBH, if this slowdown is what it is, I am comfortable with the high precision as default. Some of the tests seem to fail due to lower error bounds (i.e. the smoke test), I am not sure if that is the system lib or not, but I guess just means we need to relax those bounds slightly. This test is failing due to a compiler error because inlining failed? |
We benchmark 10000 elements. Let me try increasing the array size. But i suspect the numbers wont be too different.
I agree. 20-30% speed isn't worth all the fuss about accuracy.
4 ULP tanh was converted to universal intrinsic and we aren't using SVML for now. I was thinking I could backport SVML to universal intrinsic but it might need more time than I thought.
Not sure, will spend some time today/tomorrow in fixing up the CI failures. |
|
I think I will defer sin, cos and tanh to another PR. I will try and convert sin/cos HA SVML to universal intrinsics, which might take me some time. |
|
huh, it looks like libm float64 |
|
The CI looks stuck? |
|
The pyodide error looks unrelated. @hoodmane Could you take a look? |
|
Thanks @r-devulap . |
|
Definitely looks like it's our fault in some way. Probably we needed to pin pydantic, looks like they released a breaking change and we don't have a pin on them. Pinging more Pyodide people who might have a better idea about this: @rth @ryanking13 |
|
Opened #24091. |
Todo:
Add sin, cos and tanh