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

refine random related operators #3596

Merged
merged 8 commits into from
Aug 22, 2017

Conversation

QiJune
Copy link
Member

@QiJune QiJune commented Aug 21, 2017

Fix #3598

Copy link
Contributor

@dzhwinter dzhwinter left a comment

Choose a reason for hiding this comment

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

so, we decide to implement all the random methods in thrust?

Not a good news, we need to implement more generator in thrust by ourselves such as random_gama, ramdom_guassian
tf.random_normal
tf.truncated_normal
tf.random_uniform
tf.random_shuffle
tf.random_crop
tf.multinomial
tf.random_gamma

// And we need a global random seed configuration.
int seed = context.op_.GetAttr<int>("seed");
unsigned int seed =
static_cast<unsigned int>(context.op_.GetAttr<int>("seed"));
Copy link
Contributor

Choose a reason for hiding this comment

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

it is useless to convert int to unsigned int. Attribute Member can not contain an unsigned int.

Copy link
Member Author

Choose a reason for hiding this comment

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

And maybe we can add unsigned to attribute map

Copy link
Contributor

@dzhwinter dzhwinter left a comment

Choose a reason for hiding this comment

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

LGTM++.
we discussed offline and reach an agreement.
1, use cuda built-in need to implement the odd and even number generator.
2, it's really time cost to create a curandom generator.

@QiJune QiJune merged commit 7443b2e into PaddlePaddle:develop Aug 22, 2017
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.

None yet

2 participants