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

Rename team policy set_scratch_size. #195

Closed
vbrunini opened this issue Feb 18, 2016 · 3 comments
Closed

Rename team policy set_scratch_size. #195

vbrunini opened this issue Feb 18, 2016 · 3 comments
Assignees
Labels
Enhancement Improve existing capability; will potentially require voting
Milestone

Comments

@vbrunini
Copy link
Contributor

The fact that:

Kokkos::TeamPolicy policy(size, Kokkos::AUTO);
policy.set_scratch_size(0, Kokkos::PerTeam(bytes_per_team), Kokkos::PerThread(bytes_per_thread));
Kokkos::parallel_for(policy, /* lambda omitted */);

will not have the scratch space allocated inside the lambda is quite confusing as a user. If the requirement that the return value of the set_scratch_size() call is used for the policy passed to the parallel_for remains I think it would be more clear to name it get_policy_with_scratch_size() or something that makes it clear it is not setting member data on the existing policy.

@hcedwar hcedwar added the Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos) label Feb 18, 2016
@hcedwar
Copy link
Contributor

hcedwar commented Feb 18, 2016

Agreed. Policy parameters should be defined at construction.

@hcedwar hcedwar added this to the Summer CleanUP milestone Mar 17, 2016
@hcedwar hcedwar modified the milestones: Backlog, Summer 2016 Jun 15, 2016
@crtrott crtrott modified the milestones: Fall 2016, Backlog Sep 19, 2016
@crtrott crtrott self-assigned this Sep 19, 2016
@crtrott crtrott added the Enhancement Improve existing capability; will potentially require voting label Sep 19, 2016
@hcedwar hcedwar removed the Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos) label Sep 21, 2016
@hcedwar
Copy link
Contributor

hcedwar commented Sep 21, 2016

Issue will be resolved with refactored API to move all options in constructor.

@crtrott
Copy link
Member

crtrott commented Sep 28, 2016

Replaced by #453

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improve existing capability; will potentially require voting
Projects
None yet
Development

No branches or pull requests

3 participants