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

[REVIEW] mdspanify raft::random functions uniformInt, normalTable, fill, bernoulli, and scaled_bernoulli #897

Conversation

mhoemmen
Copy link
Contributor

@mhoemmen mhoemmen commented Oct 5, 2022

Add mdspan overloads of raft::random functions uniformInt, normalInt, normalTable, fill, bernoulli, and scaled_bernoulli. Improve normalTable documentation to explain that the output table has a row-major layout.

This is rebased atop PR #896, which I've since closed.

This should complete all the raft::random mdspan overloads.

and a unit test for it.
and a unit test for it.
There's no unit test specifically for this function,
but it is used in the normalTable test,
so I've made the mdspan version of the normalTable test
call this function.
@mhoemmen mhoemmen changed the title mdspanify raft::random functions uniformInt, normalTable, fill, bernoulli, and scaled_bernoulli [WIP] mdspanify raft::random functions uniformInt, normalTable, fill, bernoulli, and scaled_bernoulli Oct 5, 2022
@mhoemmen mhoemmen marked this pull request as ready for review October 5, 2022 20:12
@github-actions github-actions bot added the cpp label Oct 5, 2022
@mhoemmen mhoemmen requested a review from a team as a code owner October 5, 2022 20:12
@mhoemmen mhoemmen added feature request New feature or request non-breaking Non-breaking change labels Oct 5, 2022
@mhoemmen mhoemmen changed the title [WIP] mdspanify raft::random functions uniformInt, normalTable, fill, bernoulli, and scaled_bernoulli [REVIEW] mdspanify raft::random functions uniformInt, normalTable, fill, bernoulli, and scaled_bernoulli Oct 5, 2022
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

LGTM other than the small doxygen naming.

cjnolet and others added 3 commits October 5, 2022 16:50
This touches a test file changed by PR rapidsai#802,
but this commit should not conflict with that PR.
@cjnolet
Copy link
Member

cjnolet commented Oct 6, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 7bbae13 into rapidsai:branch-22.10 Oct 6, 2022
@mhoemmen mhoemmen deleted the mdspanify-fill-bernoulli-scaled_bernoulli branch October 6, 2022 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp feature request New feature or request non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants