pykokkos icon indicating copy to clipboard operation
pykokkos copied to clipboard

Update reducer types allowed in parallel reduction.

Open pkestene opened this issue 1 year ago • 2 comments

Trying to fix #220

pkestene avatar Dec 01 '23 09:12 pkestene

The changes look good. Would it be possible to add a test that confirms that this is working fine? I think you can extend this test

https://github.com/kokkos/pykokkos/blob/01f920f266444aac5101aa62bf03294791d1b256/tests/test_parallelreduce.py#L79

by simply parameterizing it on the other data types you added.

Sure, I'll update the test.

But just to clarify beforehand: in https://github.com/kokkos/pykokkos/blob/01f920f266444aac5101aa62bf03294791d1b256/tests/test_parallelreduce.py#L58 the iterate variable is a float, but should really be an integral type. I was even surprised the test builds. I think, kokkos should check that internally, no ?

pkestene avatar Dec 01 '23 22:12 pkestene

the iterate variable is a float, but should really be an integral type. I was even surprised the test builds. I think, kokkos should check that internally, no ?

Looking at the generated C++, it seems that PyKokkos ignores the type supplied by the user and generates an int32_t. In Kokkos, it is possible to set the index type through a RangePolicy template argument, but we do not support that yet. I think the test should be changed to an int iterate variable for now.

NaderAlAwar avatar Dec 04 '23 16:12 NaderAlAwar