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

Implement Robust orientation #18

Merged
merged 10 commits into from
Jul 30, 2024
Merged

Implement Robust orientation #18

merged 10 commits into from
Jul 30, 2024

Conversation

dgleich
Copy link
Collaborator

@dgleich dgleich commented Jun 21, 2024

This directly includes the AdaptivePredicates.jl code for robust orientation.

Also, this gives a speed improvement!

@vchuravy
Copy link

How much of a speedup did it actually give?

@dgleich
Copy link
Collaborator Author

dgleich commented Jun 24, 2024

Interesting, this shows a different between the Intel proc and Apple procs!

@Btime triangulate(pts2) setup=begin pts2=rand(StableRNG(1), Point2f, 1_000_000); end

On my Intel iMac Pro (Julia 1.10.2):

842 ms for using the code from AdaptivePredicates.jl
859 ms for using the code on master (orient ifsure)

On my M2 MacBook Air (Julia 1.10.1):

469ms using the code from AdaptivePredicates.jl
453ms using the code on master (orient ifsure)

@dgleich
Copy link
Collaborator Author

dgleich commented Jul 30, 2024

Hey @DanielVandH, I edited and incorporated your ported incircle test here too from Adaptive Predicates. Any objection to including it directly here? I like to try and avoid dependencies if possible. I added you to the list of people as authors of the robust.jl file.

Adding the robust incircle test does give a small overall slowdown. (more like 900ms on Intel now now; more like ~500ms now on Mac).

@DanielVandH
Copy link
Member

DanielVandH commented Jul 30, 2024

I've no problem with you including it directly, that's fine. I've done similar copies in DelaunayTriangulation.jl for avoiding other dependencies.

I like what you did with trying to avoid caching for as long as possible. I wanted to do that initially but.. lazy. The slowdown isn't too surprising but it's a nice middle ground between exact and inexact I'd say. Is your benchmark actually over a million points as you write above? I tried that and get a much longer time (though I am on an extremely crappy machine :) )

@dgleich
Copy link
Collaborator Author

dgleich commented Jul 30, 2024

Thanks. I added the performance test to this pull request to see what the CI machines give.

@DanielVandH
Copy link
Member

Oh I see. I just ran it again and get a decent result (~1.2s). Last time I got like 45 s.. don't know what happened there. Must've got unlucky. That's an impressive speed! Is it using spatial sorting?

@dgleich dgleich merged commit c066b04 into master Jul 30, 2024
5 checks passed
@dgleich dgleich deleted the robust branch July 30, 2024 19:55
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.

3 participants