Rework ops.random.Uniform #2531
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
jantonguirao
force-pushed
the
uniform_rng_rework
branch
from
5c7c8ce
to
3125063
Compare
Signed-off-by: Joaquin Anton <janton@nvidia.com>
jantonguirao
force-pushed
the
uniform_rng_rework
branch
from
3125063
to
397e259
Compare
Signed-off-by: Joaquin Anton <janton@nvidia.com>
jantonguirao
force-pushed
the
uniform_rng_rework
branch
from
98e1169
to
63966f9
Compare
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
jantonguirao
changed the title
jantonguirao
changed the title
|
!build |
|
CI MESSAGE: [1940512]: BUILD STARTED |
|
CI MESSAGE: [1940512]: BUILD FAILED |
Signed-off-by: Joaquin Anton <janton@nvidia.com>
jantonguirao
force-pushed
the
uniform_rng_rework
branch
from
4440ba6
to
7e3b443
Compare
|
!build |
|
CI MESSAGE: [1940603]: BUILD STARTED |
|
CI MESSAGE: [1940603]: BUILD PASSED |
szalpal
reviewed
Jan 8, 2021
dali/operators/util/randomizer.cuh
Outdated
|
|
||
| template <typename T> | ||
| struct curand_uniform_dist {}; | ||
|
|
There was a problem hiding this comment.
How about creating
template<typename T>
struct curand_uniform_dist {
static_assert( /* check if T is double or float */ );
DALI_HOST_DEV curand_uniform_dist(T start, T end)
/* etc... */
};And specializing only curand_uniform/curand_uniform_double? This way we have this extra static verification of type with meaningful error msg (and less code duplication). Cause after
curand_uniform_dist<int> d;We're getting something peculiar I guess
There was a problem hiding this comment.
Fixed. I changed it to a forward declaration, so it won't compile for other types.
dali/operators/util/randomizer.cuh
Outdated
|
|
||
| struct curand_uniform_int_range_dist { | ||
| DALI_HOST_DEV curand_uniform_int_range_dist(int start, int end) | ||
| : range_start_(start), range_size_(end-start) {} |
There was a problem hiding this comment.
How about adding assert(start < end) line?
dali/operators/util/randomizer.cuh
Outdated
| template <> | ||
| struct curand_uniform_dist<double> { | ||
| DALI_HOST_DEV curand_uniform_dist(float start, float end) | ||
| : range_start_(start), range_size_(end-start) {} |
Signed-off-by: Joaquin Anton <janton@nvidia.com>
banasraf
reviewed
Jan 11, 2021
dali/operators/util/randomizer.cuh
Outdated
| struct curand_uniform_dist {}; | ||
|
|
||
| template <> | ||
| struct curand_uniform_dist<float> { |
There was a problem hiding this comment.
As Michał suggested, you can move the common parts to the same place and specialize only the operator().
banasraf
reviewed
Jan 11, 2021
| .DocStr(R"code(Generates random numbers following a uniform distribution. | ||
|
|
||
| The shape of the generated data can be either specified explicitly with a ``shape`` argument, | ||
| or chosen to match the shape of the input, if provided. If none are present, a single number is |
There was a problem hiding this comment.
A single number means a batch of scalars, right? If so, maybe it could be phrased this way in the doc str.
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
banasraf
approved these changes
Jan 11, 2021
szalpal
approved these changes
Jan 12, 2021
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
|
!build |
|
CI MESSAGE: [1973816]: BUILD STARTED |
|
CI MESSAGE: [1973816]: BUILD PASSED |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why we need this PR?
Pick one, remove the rest
What happened in this PR?
Fill relevant points, put NA otherwise. Replace anything inside []
Implemented ops.random.Uniform in terms of RNGBase
Moved ops.Uniform to ops.random.Uniform
ops.random.Uniform
Uniform implementation
Tests extended
Existing documentation
JIRA TASK: [DALI-1196]