Skip to content
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

5.5x faster, half as many divisions, recovering accuracy #99

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

PallHaraldsson
Copy link
Contributor

No description provided.

@PallHaraldsson PallHaraldsson changed the title 3.8x faster, recovering accuracy 5.5x faster, half as many divisions, recovering accuracy Nov 15, 2022
@niklas-heer
Copy link
Owner

@PallHaraldsson I appreciate your effort trying to improve the Julia code.
It seems that the compilation version of Julia is failing.
https://github.com/niklas-heer/speed-comparison/actions/runs/3496877056/jobs/5855300425#step:6:2638

Maybe this is related to the blocker in #57?

I don't know if this line is needed, those are the (latest) package versions I used (assume 0.5 works too):

⌃ [81625895] StaticCompiler v0.4.12
  [86c06d3c] StaticTools v0.8.8

I used Julia 1.9.0, not (that should also work and 1.8.5, doubt any have speed advantage):
```
FROM julia:1.8.2
```
@PallHaraldsson
Copy link
Contributor Author

Hi @niklas-heer, I believe this is done now finally after a long wait, and the speed maintained hopefully, after a fix for StaticCompiler.jl.

A "sufficiently smart compiler" could in theory figure out my transformation, but I very much doubt any will (then would do already), or will come soon, since the optimization isn't very general-purpose?

Why is "Julia (ux4)" marked specially in the graph? Was it at some point the fastest Julia version? Or some other reason for it? Maybe just drop it, in a later PR?

Copy link
Owner

@niklas-heer niklas-heer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@PallHaraldsson
Copy link
Contributor Author

Please hold of on merging. I believe I timed 5x faster for sure originally (in Julia REPL), but StaticCompiler.jl couldn't handle that code. Now after fixing it seems slower, I need to see why, and for sure compare to all the right versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants