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

About atomic views in example 05_simple_atomics #219

Open
pkestene opened this issue Nov 30, 2023 · 1 comment
Open

About atomic views in example 05_simple_atomics #219

pkestene opened this issue Nov 30, 2023 · 1 comment

Comments

@pkestene
Copy link
Contributor

This is just a remark/suggestion:

  • currently the top level README.md recommends to build pykokkos-base with option ENABLE_MEMORY_TRAITS=OFF
  • doing so, when running 05_simple_atomics.py it fails with the following error message
Traceback (most recent call last):
  File "/home/kestenerp/install/kokkos/github/pykokkos_pk/examples/kokkos/05_simple_atomics.py", line 48, in <module>
    pk.execute(pk.ExecutionSpace.OpenMP, SimpleAtomics(100))
                                         ^^^^^^^^^^^^^^^^^^
  File "/home/kestenerp/install/kokkos/github/pykokkos_pk/examples/kokkos/05_simple_atomics.py", line 14, in __init__
    self.count: pk.View1D[pk.int32] = pk.View([1], pk.int32, trait=pk.Trait.Atomic)
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kestenerp/install/kokkos/github/pykokkos_pk/pykokkos/interface/views.py", line 231, in __init__
    self._init_view(shape, dtype, space, layout, trait, array)
  File "/home/kestenerp/install/kokkos/github/pykokkos_pk/pykokkos/interface/views.py", line 337, in _init_view
    self.array = kokkos_lib.array("", shape, None, None, self.dtype.value, space.value, layout.value, trait.value)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kestenerp/miniconda3/envs/pykokkos/lib/python3.11/site-packages/pykokkos_base-0.0.7-py3.11-linux-x86_64.egg/kokkos/utility.py", line 201, in array
    return getattr(lib, _name)(label if array is None else array, shape)
           ^^^^^^^^^^^^^^^^^^^
AttributeError: module 'kokkos.libpykokkos' has no attribute 'KokkosView_int32_HostSpace_LayoutRight_Atomic_1'. Did you mean: 'KokkosView_int32_HostSpace_LayoutRight_1'?

But I have the feeling that pykokkos could first check that pykokkos-base actually enable memory trait, and if not enabled, make the error message more explicit and tell the user: "Please rebuild pykokkos-base with memory trait enabled"

@NaderAlAwar
Copy link
Contributor

In the Kokkos wiki page for views, the recommendation is to specify memory traits as late as possible. It shouldn't be necessary to specify the memory trait when calling the view constructor (as pykokkos does now), rather it should be done on a per-kernel basis.

In other words, we should never try to build pykokkos-base with memory traits enabled. If a user asks for a particular memory trait, we should not call the constructor with that memory trait. Instead, we will handle it during code generation, specifically when we generate the C++ Kokkos functor and its member variables.

More concretely, to fix this, we will need two things:

  1. User interface: we need to decide how to specify a certain memory trait. I think the right way to do this is to remove it from the constructor and have a separate method that returns an alias of the view with a specific trait. For example, if count is a View, we could maybe do count_atomic = count.as_atomic(). This will return a wrapper of the same view with a member variable specifying that it has the atomic trait.
  2. Code generation: during code generation, we need to modify functor.py to recognize views with custom memory traits and generate the right code accordingly.

I would be happy to accept and assist with a PR that fixes this. Otherwise, this will have to wait some time as I am currently busy working on other features.

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

No branches or pull requests

2 participants