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

Always clear threadToQpuId when we clear platformQPUs #2478

Merged
merged 4 commits into from
Dec 13, 2024

Conversation

1tnguyen
Copy link
Collaborator

Description

We should clear threadToQpuId when we initialize the platformQPUs list (clear then repopulate).
The runtime has a static quantum platform instance, hence there could be stray info between run.

For example, a previous run with nvidia-mqpu may populate threadToQpuId based on the number of GPUs. Then, a subsequent remote-mqpu run may have a collision when looking up threadToQpuId with thread id => out of bound access to the platformQPUs map.

This is primarily a Python specific bug as we cannot switch targets with C++.

Signed-off-by: Thien Nguyen <thiennguyen@nvidia.com>
Copy link
Collaborator

@bmhowe23 bmhowe23 left a comment

Choose a reason for hiding this comment

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

This is a good find, Thien. Do we also need similar changes here?

(Those changes also do platformQPUs.clear() and would therefore seem to invalidate threadToQpuId, if it existed.

Copy link

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

github-actions bot pushed a commit that referenced this pull request Dec 13, 2024
@1tnguyen
Copy link
Collaborator Author

This is a good find, Thien. Do we also need similar changes here?

(Those changes also do platformQPUs.clear() and would therefore seem to invalidate threadToQpuId, if it existed.

Good point. Added in b0b5f4a.

I think for DefaultQuantumPlatform, this will not cause a problem as we only expect a single qpu (qpu_id = 0) but it's good to have both the vector and the map cleared at the same time throughout the code.

Copy link
Collaborator

@bmhowe23 bmhowe23 left a comment

Choose a reason for hiding this comment

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

👍

github-actions bot pushed a commit that referenced this pull request Dec 13, 2024
Copy link

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

@schweitzpgi schweitzpgi enabled auto-merge (squash) December 13, 2024 19:24
@schweitzpgi schweitzpgi merged commit de6699d into NVIDIA:main Dec 13, 2024
211 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 13, 2024
Copy link

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

@bettinaheim bettinaheim added testing Relates to testing no release notes Don't list this PR in the release notes labels Dec 17, 2024
@bettinaheim bettinaheim added this to the release 0.9.2 milestone Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no release notes Don't list this PR in the release notes testing Relates to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants