Skip to content

Fix nested workunits - #435

Open
WeiTaShao wants to merge 3 commits into
kokkos:mainfrom
WeiTaShao:fix-nested-workunits
Open

Fix nested workunits#435
WeiTaShao wants to merge 3 commits into
kokkos:mainfrom
WeiTaShao:fix-nested-workunits

Conversation

@WeiTaShao

Copy link
Copy Markdown
Contributor

Implicit helper functions

Standalone PyKokkos workunits can call compatible module-level Python functions without requiring the @pk.function decorator. PyKokkos discovers helper functions reachable from the workunit, recursively includes their same-module dependencies in translation, and excludes unrelated host-side functions.

import pykokkos as pk


def my_printer(i: int) -> None:
    pk.printf("Index: %d\n", i)


@pk.workunit
def hello(i: int) -> None:
    my_printer(i)


pk.parallel_for("hello_loop", 10, hello)

@IvanGrigorik IvanGrigorik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

self.add_called_functions is highly questionable.
I'm not sure if this is the easiest way to check which function is called.

Instead of gathering all possible functions (which is insanely heavy) we can check which function is called based on the current context directly from the AST translator.


And yes, don't remote @pk.function decorator nor ignore it. It will be a pain if we will try to visit each function without looking at its source of it.

Comment thread pykokkos/core/compiler.py
self,
path: List[str],
name: str,
ast_signature: str,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So...
Now you will recompile if ANYTHING in AST changed?
That's questionable.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please add a test of defining a function in one module (file) and using it from another module workunit.

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.

2 participants