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

boolean reduction value_type changes answer #225

Closed
ibaned opened this issue Mar 25, 2016 · 5 comments
Closed

boolean reduction value_type changes answer #225

ibaned opened this issue Mar 25, 2016 · 5 comments
Labels
Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos)
Milestone

Comments

@ibaned
Copy link
Contributor

ibaned commented Mar 25, 2016

I'm trying to write some custom parallel_reduce functors which will compute whether a condition is true for all items in an array. I tried to use "bool" as the value_type and implement methods based on the && operator. Strangely, it gives the wrong output when value_type is bool but I can fix it by using an integer value_type:

struct SameContent {
#if 0
  typedef int value_type;
#else
  typedef bool value_type;
#endif
  Kokkos::View<int*> a_;
  Kokkos::View<int*> b_;
  SameContent(Kokkos::View<int*> a, Kokkos::View<int*> b):a_(a),b_(b) {}
  KOKKOS_INLINE_FUNCTION
  void init(value_type& update) const
  {
    update = 1;
  }
  KOKKOS_INLINE_FUNCTION
  void join(volatile value_type& update,
      const volatile value_type& input) const
  {
    update = update && input;
  }
  KOKKOS_INLINE_FUNCTION
  void operator()(int i, value_type& update) const
  {
    update = update && (a_[i] == b_[i]);
  }
};

Am I misusing Kokkos here ?
If this is something that should work, I have a two-file standalone example that replicates the issue. All the above is using Kokkos with CUDA on Shannon.

@hcedwar
Copy link
Contributor

hcedwar commented Mar 25, 2016

Reduction value_type needs to be a word-aligned plain-old-data types. Unfortunately 'bool' can be compiled to a fraction of a word, which is probably the source of the error. We should introduce a static_assert to catch / enforce this requirement.

@ibaned
Copy link
Contributor Author

ibaned commented Mar 25, 2016

Thank you for the explanation. I'll switch to word-sized value_types in my code. A compile-time check would be much appreciated.

@ibaned ibaned closed this as completed Mar 25, 2016
@hcedwar
Copy link
Contributor

hcedwar commented Mar 25, 2016

The types can be larger than a word (i.e., int) just not smaller. Re-opening until we get the static_assert into the code, then will close.

@hcedwar hcedwar reopened this Mar 25, 2016
@hcedwar hcedwar added the Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos) label Mar 25, 2016
@hcedwar hcedwar added this to the GTC 2016 milestone Mar 29, 2016
hcedwar added a commit that referenced this issue Mar 29, 2016
	0 == sizeof(value_type) % sizeof(int)

Closes issue #225 and duplicate issue #226 .
@crtrott
Copy link
Member

crtrott commented Apr 1, 2016

Pushed to Master and Trilinos: you can not do reductions with types smaller than 32bit now

@crtrott crtrott closed this as completed Apr 1, 2016
@ibaned
Copy link
Contributor Author

ibaned commented Jul 25, 2016

Just FYI, I had to increase this further from int32_t to int64_t to use an NVidia GTX 980 Ti with the CUDA 8.0 RC.

ibaned/omega_h@18110db

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos)
Projects
None yet
Development

No branches or pull requests

3 participants