Skip to content

Added missing GIL-safe implementation for pySampleAsync - #828

Merged
1tnguyen merged 5 commits into
NVIDIA:mainfrom
1tnguyen:tnguyen/issue-640
Oct 30, 2023
Merged

Added missing GIL-safe implementation for pySampleAsync#828
1tnguyen merged 5 commits into
NVIDIA:mainfrom
1tnguyen:tnguyen/issue-640

Conversation

@1tnguyen

Copy link
Copy Markdown
Collaborator

Description

Converting Python args to native data before passing it to the async sample functor. We've implemented it for observe_async, hence adopting it for pySampleAsync.

Addressed #640.

@github-actions

Copy link
Copy Markdown

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions Bot pushed a commit that referenced this pull request Oct 27, 2023

@bmhowe23 bmhowe23 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.

LGTM 👍

@amccaskey amccaskey 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.

+1

Comment thread python/runtime/cudaq/algorithms/py_sample.cpp Outdated

@schweitzpgi schweitzpgi 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.

👍

Comment thread python/runtime/cudaq/algorithms/py_sample.cpp
@copy-pr-bot

copy-pr-bot Bot commented Oct 30, 2023

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@1tnguyen

1tnguyen commented Oct 30, 2023

Copy link
Copy Markdown
Collaborator Author

/ok to test

Command Bot: Processing...

- When using kernel_builder with async. in C++, we may hit thread-safety
  issue in jit compile. For Python, we pre-jit the kernel on the
main thread hence is not affected by this.

- Fix issues with get_state_async: missing qpu_id in reset_context and
  set_current_qpu.

- Update comments.
@1tnguyen

1tnguyen commented Oct 30, 2023

Copy link
Copy Markdown
Collaborator Author

I've updated this PR to add some more fixes also related to async. execution. Please help re-review it.

@1tnguyen

1tnguyen commented Oct 30, 2023

Copy link
Copy Markdown
Collaborator Author

/ok to test

Command Bot: Processing...

@1tnguyen

1tnguyen commented Oct 30, 2023

Copy link
Copy Markdown
Collaborator Author

/ok to test

Command Bot: Processing...

@1tnguyen

1tnguyen commented Oct 30, 2023

Copy link
Copy Markdown
Collaborator Author

/ok to test

Command Bot: Processing...

@github-actions

Copy link
Copy Markdown

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions Bot pushed a commit that referenced this pull request Oct 30, 2023

@amccaskey amccaskey 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.

new changes look good to me

@1tnguyen

1tnguyen commented Oct 30, 2023

Copy link
Copy Markdown
Collaborator Author

/ok to test

Command Bot: Processing...

@github-actions

Copy link
Copy Markdown

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions Bot pushed a commit that referenced this pull request Oct 30, 2023
@1tnguyen
1tnguyen merged commit 620efd1 into NVIDIA:main Oct 30, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 30, 2023
@bettinaheim bettinaheim added the bug fix To be listed under Bug Fixes in the release notes label Nov 15, 2023
@bettinaheim bettinaheim added this to the release 0.5.0 milestone Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug fix To be listed under Bug Fixes in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants