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

Add RNG to make deterministic #35

Merged
merged 4 commits into from
Apr 6, 2023
Merged

Conversation

skygering
Copy link
Contributor

Solution to #34

My pull request to VoronoiDelauny was merged. My PR allows us to seed the call to random! that was causing the non-deterministic behavior, as the different orders of operations were leading to differences in the floating point answer.

Here I have made an optional RNG argument to the call to voronoicells. My tests show that if you provide RNGs with different seeds, voronoicells gives two different answers, and that if we provide the same seed we get the same answer.

This requirers VoronoiDelauny v0.4.4, which I changed in the Project.toml.

@codecov
Copy link

codecov bot commented Apr 6, 2023

Codecov Report

Merging #35 (4b7cfd4) into master (10dd2b9) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #35   +/-   ##
=======================================
  Coverage   89.38%   89.38%           
=======================================
  Files           7        7           
  Lines         226      226           
=======================================
  Hits          202      202           
  Misses         24       24           
Impacted Files Coverage Δ
src/Cells.jl 88.52% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@robertdj robertdj merged commit 83f2658 into JuliaGeometry:master Apr 6, 2023
@Wikunia
Copy link

Wikunia commented Oct 10, 2023

Would be great to add this to the readme as I think a lot of developers will expect a deterministic behaviour if not mentioned otherwise.

@Wikunia Wikunia mentioned this pull request Oct 10, 2023
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