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

Fix logic to find real module path #2339

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

boschmitt
Copy link
Collaborator

Description

With python device kernel interoperability, users can write quantum kernels in C++ and bind them to python. In such cases, the common pattern is to have a C++ module that gets imported into a python module.

For example, if we have a python package named foo to which we add C++ extensions using pybind11. The common pattern is to end up with a with a module named _cppfoo (or whaterver). Then, we import all of its symbols to foo:

foo/__init__.py:
  from ._cppfoo import *

Now, if _cppfoo contains a binded device kernel named bar, then users are able to access it using foo.bar(...). This, however, is not the real path of bar, the real path is foo._cppfoo.bar(..).

Curently, binded device kernels get registered with their real path name, and thus when the python AST bridge parse another kernel that uses foo.bar(...), it needs to figure it out if that is its real path or not.

This commit attemps to improve the robustness of discovering this real path because as-is it fails on some simple cases.

This is how it works: In Python, many objects have a module attribute, which indicates the module in which the object was defined. This should be the case for functions. Thus the idea here is to walk the provide path until we reach the function object and ask it for its __module__.

Copy link
Collaborator

@khalatepradnya khalatepradnya left a comment

Choose a reason for hiding this comment

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

Changes look good to me. Thanks, Bruno!
(Open Q on where / how to test)

@boschmitt
Copy link
Collaborator Author

Changes look good to me. Thanks, Bruno! (Open Q on where / how to test)

Thanks @khalatepradnya. I open an issue, #2348, on this. There I created a patch that breaks the current implementation. Unfortunately, my current fix is not working, it breaks other corner cases. I will keep looking more into to it.

@boschmitt boschmitt force-pushed the fix_fiding_real_path branch 2 times, most recently from ea0b81e to b913301 Compare November 1, 2024 17:18
With python device kernel interoperability, users can write quantum
kernels in C++ and bind them to python. In such cases, the common
pattern is to have a C++ module that gets imported into a python module.

For example, if we have a python package named `foo` to which we add
C++ extensions using pybind11. The common pattern is to end up with a
with a module named `_cppfoo` (or whaterver). Then, we import all of its
symbols to `foo`:

foo/__init__.py:
  from ._cppfoo import *

Now, if `_cppfoo` contains a binded device kernel named `bar`, then
users are able to access it using `foo.bar(...)`. This, however, is not
the real path of `bar`, the real path is `foo._cppfoo.bar(..)`.

Currently, binded device kernels get registered with their real path
name, and thus when the python AST bridge parse another kernel that
uses `foo.bar(...)`, it needs to figure it out if that is its real path
or not.

This commit attemps to improve the robustness of discovering this real
path because as-is it fails on some simple cases.

This is how it works: In Python, many objects have a module attribute,
which indicates the module in which the object was defined. This should
be the case for functions. Thus the idea here is to walk the provide
path until we reach the function object and ask it for its `__module__`.

Signed-off-by: boschmitt <[email protected]>
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