ENH: array API start #57

tylerjereddy · 2022-08-09T23:44:42Z

Related to #56. This at least allows us to run the array API conformance test suite, rather than erroring out at import time. I suspect some of the changes here we won't really want to make, but on the flip side this gives folks an idea of some of the changes I needed to get us even remotely close to being able to run the suite.

support the bare minimum required types per:
https://data-apis.org/array-api/2021.12/API_specification/data_types.html
(otherwise, we can't even import the array API tests, let
alone run them--for now, some types are basically just "stubs"
and/or aliases so we can get things rolling)
support iinfo and finfo functions required by the standard:
https://data-apis.org/array-api/2021.12/API_specification/data_type_functions.html#objects-in-api
needed to add a (hacky) implemention of asarray() to comply
with the array API standard per:
https://data-apis.org/array-api/2021.12/API_specification/creation_functions.html#objects-in-api
Note that the positional-only and keyword-only function signature
is also mandatory.
the array API test suite detected multiple issue in our data type
system; these mostly seemed to stem from having both DataType(Enum)
and DataTypeClass, which is a model that is not consistent with
the mappings expected by the standard; it is also mandatory to support
more types than we currently do, so I've hacked around some of these
issues for now, but with boolean type we'll currently fail the array API tests
we'll need to double check what to do for 0 dimensional arrays, but
they are used in the array API test suite, so I've added a temporary
hack around those until we support them "natively?"
add a CI job that starts to test for array API compliance--it will
fail, but it should at least start running rather than erroring out
at the import stage, which is a step forward from develop branch
the usual mypy ignores, at least for now..

* support the bare minimum required types per: https://data-apis.org/array-api/2021.12/API_specification/data_types.html (otherwise, we can't even import the array API tests, let alone run them--for now, some types are basically just "stubs" and/or aliases so we can get things rolling) * support `iinfo` and `finfo` functions required by the standard: https://data-apis.org/array-api/2021.12/API_specification/data_type_functions.html#objects-in-api * needed to add a (hacky) implemention of `asarray()` to comply with the array API standard per: https://data-apis.org/array-api/2021.12/API_specification/creation_functions.html#objects-in-api Note that the positional-only and keyword-only function signature is also mandatory. * the array API test suite detected multiple issue in our data type system; these mostly seemed to stem from having both `DataType(Enum)` and `DataTypeClass`, which is a model that is not consistent with the mappings expected by the standard; it is also mandatory to support more types than we currently do, so I've hacked around some of these issues for now, but with boolean type we'll currently fail the array API tests * we'll need to double check what to do for `0` dimensional arrays, but they are used in the array API test suite, so I've added a temporary hack around those until we support them "natively?" * add a CI job that starts to test for array API compliance--it will fail, but it should at least start running rather than erroring out at the import stage, which is a step forward from `develop` branch * the usual mypy ignores, at least for now..

* more changes to improve behavior with array API test suite

tylerjereddy · 2022-08-11T22:47:18Z

pykokkos/interface/views.py

@@ -417,7 +449,16 @@ def from_numpy(array: np.ndarray, space: Optional[MemorySpace] = None, layout: O
    if layout is None:
        layout = Layout.LayoutDefault

-    return View(list(array.shape), dtype, space=space, trait=Trait.Unmanaged, array=array, layout=layout)
+    # TODO: pykokkos support for 0-D arrays?
+    # temporary/terrible hack here for array API testing..


So, the pykokkos tests proper are passing in this PR now. Of course, even the small subset of array API tests I've added in CI don't pass yet. There appear to be two main challenges:

Lack of native support for bool in Kokkos proper? I asked about this on the main Slack channel and there didn't seem to be any suggestions.

What we currently have exposed for 0-d array support. There were some comments on Slack which I'll just paste below:

Kokkos has rank-0 View which contains a single scalar value. It doesn’t have a unit-like View or whatever

yeah a rank-0 view is also accessible as you suggested by ()

View<int> a("A"); a() = 5;

mdspan is the same (just with [] operator instead)

mdspan<int, dextents<0>> a(ptr_a); a[] = 5;

* adjust the new array API CI job to only run tests that pass with the current state of the PR; this is still progress because we can't even import the array API tests on `develop` let alone pass some of them

tylerjereddy · 2022-08-17T20:02:40Z

@NaderAlAwar Ok, the CI is passing now, so this is probably ready for review. I'll summarize what's going on here below:

I ended up selecting to run only a small subset of the array API testsuite in CI to start--this is still progress in the sense that if you try to run the array API testsuite on develop, you won't even be able to import the tests let alone run/pass them because of the dtype system inconsistencies
some of the changes still don't feel quite right, so those cases may also reflect situations where the current pykokkos testsuite isn't catching some stuff? see the selective pass here for example: https://github.com/kokkos/pykokkos/pull/57/files#diff-a471f687656d8dda27306cb2a3a4ec31fa9b182df23b5dbd72a563ac309ee83cR275
we may want to open issues for some of the discussion points I made above re: bool and 0-D array support; conversely, there are so many TODOs left on the array API conformance that opening issues for them all may be a bit much anyway for now
I didn't have to modify any pykokkos tests proper, so that's probably a good sign
there are probably a few extraneous imports in the diff, from a quick scan of my work

NaderAlAwar

The changes look good. I think eventually we need to clean up the data type issue where we have both the enum and the empty classes and address the bool and 0d array issues, but we can merge this now if you want.

NaderAlAwar · 2022-08-18T18:28:38Z

pykokkos/__init__.py

@@ -19,6 +19,9 @@
                                 log1p,
                                 sqrt,
                                 sign)
+from pykokkos.lib.info import iinfo, finfo
+from pykokkos.lib.create import zeros
+from pykokkos.lib.util import all, any


It's not clear to me what exactly is supposed to go under lib/. Is it supposed to be any code that we write that also uses pykokkos?

NaderAlAwar · 2022-08-18T18:46:29Z

pykokkos/interface/data_types.py

+    # now, we will use aliases and possibly even incorrect/empty
+    # implementations so that we can start testing with the array
+    # API standard suite, otherwise we won't even be able to import
+    # the tests let alone run them


This should be fine for now, but we will need to clean up the datatypes later

* add support for 8-bit signed integers with values that should exist on the interval `[-128, 127]` for conformance with the array API: https://data-apis.org/array-api/latest/API_specification/data_types.html kokkos/pykokkos#57 * add a regression test for kokkos<->NumPy type equivalencies that includes the new `int8` type; I was hoping to do some simple arithmetic testing as well, but not sure how practical that is within `pykokkos-base` proper (maybe defer that kind of thing to `pykokkos`?)

tylerjereddy · 2022-08-25T18:28:14Z

Nader indicated that this was "ok" to merge as-is at the meeting today, and CI is passing.

If nothing else, this solidifies some progress on the array API conformance front and enforces it in the CI.

* add support for 8-bit signed integers with values that should exist on the interval `[-128, 127]` for conformance with the array API: https://data-apis.org/array-api/latest/API_specification/data_types.html kokkos/pykokkos#57 * add a regression test for kokkos<->NumPy type equivalencies that includes the new `int8` type; I was hoping to do some simple arithmetic testing as well, but not sure how practical that is within `pykokkos-base` proper (maybe defer that kind of thing to `pykokkos`?)

* ENH: support int8 * add support for 8-bit signed integers with values that should exist on the interval `[-128, 127]` for conformance with the array API: https://data-apis.org/array-api/latest/API_specification/data_types.html kokkos/pykokkos#57 * add a regression test for kokkos<->NumPy type equivalencies that includes the new `int8` type; I was hoping to do some simple arithmetic testing as well, but not sure how practical that is within `pykokkos-base` proper (maybe defer that kind of thing to `pykokkos`?) * MAINT: PR 43 revisions * format with `black` because of a failing CI check * use `signed_char` as the shorthand for `int8`, for similarity with: https://numpy.org/doc/stable/user/basics.types.html

tylerjereddy force-pushed the treddy_array_api_start branch from bf94c38 to a949831 Compare August 9, 2022 23:48

tylerjereddy added 2 commits August 11, 2022 15:55

MAINT: PR 57 revisions

d890bff

* more changes to improve behavior with array API test suite

tylerjereddy force-pushed the treddy_array_api_start branch from 14330c2 to d890bff Compare August 11, 2022 22:00

tylerjereddy commented Aug 11, 2022

View reviewed changes

MAINT: PR 57 revisions

fe4c515

* adjust the new array API CI job to only run tests that pass with the current state of the PR; this is still progress because we can't even import the array API tests on `develop` let alone pass some of them

NaderAlAwar reviewed Aug 18, 2022

View reviewed changes

tylerjereddy mentioned this pull request Aug 23, 2022

ENH: support int8 kokkos/pykokkos-base#43

Merged

tylerjereddy changed the title ~~WIP, ENH: array API start~~ ENH: array API start Aug 25, 2022

tylerjereddy added the enhancement New feature or request label Aug 25, 2022

tylerjereddy merged commit cf7157f into kokkos:develop Aug 25, 2022

tylerjereddy deleted the treddy_array_api_start branch August 25, 2022 18:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: array API start #57

ENH: array API start #57

tylerjereddy commented Aug 9, 2022

tylerjereddy Aug 11, 2022

tylerjereddy commented Aug 17, 2022

NaderAlAwar left a comment

NaderAlAwar Aug 18, 2022

NaderAlAwar Aug 18, 2022

tylerjereddy commented Aug 25, 2022

ENH: array API start #57

ENH: array API start #57

Conversation

tylerjereddy commented Aug 9, 2022

tylerjereddy Aug 11, 2022

Choose a reason for hiding this comment

tylerjereddy commented Aug 17, 2022

NaderAlAwar left a comment

Choose a reason for hiding this comment

NaderAlAwar Aug 18, 2022

Choose a reason for hiding this comment

NaderAlAwar Aug 18, 2022

Choose a reason for hiding this comment

tylerjereddy commented Aug 25, 2022