Skip to content

Conversation

@pkestene
Copy link
Contributor

@pkestene pkestene commented Dec 1, 2023

Trying to fix #220

Copy link
Contributor

@NaderAlAwar NaderAlAwar left a comment

Choose a reason for hiding this comment

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

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

def test_squaresum_types(series_max, dtype):

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

@pkestene
Copy link
Contributor Author

pkestene commented Dec 1, 2023

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

def test_squaresum_types(series_max, dtype):

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

Sure, I'll update the test.

But just to clarify beforehand:
in

def squaresum(self, i: float, acc: pk.Acc[pk.double]):
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 ?

@NaderAlAwar
Copy link
Contributor

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.

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.

Example random_sum.py fails because int32 is not a supported type for a reduction

2 participants