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

Features/54 distributed random numbers #362

Merged
merged 32 commits into from
Sep 12, 2019

Conversation

TheSlimvReal
Copy link
Contributor

Description

Target issue #54.

This builds on @Markus-Goetz implementation of the threefry64 algorithm. The __counter_sequenze function was completed which returns initial values for the "random" number calculation. It is currently only tested for torch.int64 because the threefry32 is not implemented yet.

  • New feature (non-breaking change which adds functionality)

heat/core/random.py Show resolved Hide resolved
heat/core/tests/test_random.py Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 2, 2019

Codecov Report

Merging #362 into master will increase coverage by 0.09%.
The diff coverage is 98.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #362      +/-   ##
==========================================
+ Coverage   97.14%   97.23%   +0.09%     
==========================================
  Files          53       53              
  Lines        9659    10093     +434     
==========================================
+ Hits         9383     9814     +431     
- Misses        276      279       +3
Impacted Files Coverage Δ
heat/core/manipulations.py 98.11% <ø> (ø) ⬆️
heat/core/tests/test_statistics.py 100% <100%> (ø) ⬆️
heat/core/random.py 100% <100%> (+10%) ⬆️
heat/core/operations.py 95.2% <100%> (ø) ⬆️
heat/core/tests/test_manipulations.py 100% <100%> (ø) ⬆️
heat/core/tests/test_communication.py 97.95% <100%> (ø) ⬆️
heat/ml/cluster/kmeans.py 100% <100%> (ø) ⬆️
heat/core/tests/test_random.py 100% <100%> (ø) ⬆️
heat/core/statistics.py 95.53% <86.36%> (-1.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e6d44c...e774c95. Read the comment docs.

Markus-Goetz
Markus-Goetz previously approved these changes Sep 6, 2019
@Markus-Goetz
Copy link
Member

matplotlib just simply needs to go still

@Markus-Goetz
Copy link
Member

What is the state regarding the 32-bit Threefry? Have you tried the upper bit removal?

@TheSlimvReal
Copy link
Contributor Author

What is the state regarding the 32-bit Threefry? Have you tried the upper bit removal?

I tried to use the 64-Bit logic for the random numbers and cut off the upper 32-Bit but this resulted in a similar probability for equal values compared to the threefry-32 algorithm. (Around 10 per 10k values)

@Markus-Goetz
Copy link
Member

Right, so let's do the following, lets use the regular threefry32 and just live with the fact that there is repetition. As for your unit tests, just set a slightly higher upper bound for the number of allowed repetitions than 1, e.g. 10?

@Markus-Goetz Markus-Goetz merged commit 39a744b into master Sep 12, 2019
@Markus-Goetz Markus-Goetz deleted the features/54-distributed-random-numbers branch September 12, 2019 12:54
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