Fix confusion between stdlib random and np.random#4645
Merged
abbycross merged 4 commits intoQiskit:mainfrom Feb 9, 2026
Merged
Fix confusion between stdlib random and np.random#4645abbycross merged 4 commits intoQiskit:mainfrom
abbycross merged 4 commits intoQiskit:mainfrom
Conversation
The existing code imports both `pi` and `random` from numpy. The notebook uses nothing else from numpy. These two objects are used only in a scalar context. In some cases, using numpy can degrade performance. In qiskit there has been an effort, reflected in PRs, to avoid using numpy habitually when not necessary. This is done even in non-performance critical code. This PR changes the code to import `pi` from `math`. And it imports `random` from the stdlib. When producing single samples, standard library `random.randint` is about 5 times faster than `numpy.random.randint`. The existing code calls numpy's `random.randint(0, 1)`, which always returns zero. This was mistakenly introduced. Since this PR uses `randint` from the standard library, the range is 0 to 1, inclusive. So it may return either 0 or 1. My best understanding is that this is the original intent of the code.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Contributor
|
One or more of the following people are relevant to this code: |
@jlapeyre a linting error flagged a json-breaking change. Does this fix preserve your intent?
abbycross
reviewed
Feb 9, 2026
| "from qiskit.result import marginal_distribution\n", | ||
| "from qiskit.circuit.library import UGate\n", | ||
| "from numpy import pi, random" | ||
| "from math import pi, random" |
Collaborator
There was a problem hiding this comment.
@jlapeyre the json was invalid - this fix now passes the linter but pls let me know if it doesn't match what is needed here
Collaborator
There was a problem hiding this comment.
I've now put import random on its own line in the notebook (vs. directly in the json), and it healed the lint problem - that looks more like what you wanted to accomplish
Collaborator
Author
There was a problem hiding this comment.
@abbycross Thankyou. This is correct. Next time, I'll run jq.
christopherporter1
approved these changes
Feb 9, 2026
Collaborator
christopherporter1
left a comment
christopherporter1
left a comment
There was a problem hiding this comment.
Thank you, John!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The existing code that generates
https://quantum.cloud.ibm.com/learning/en/courses/basics-of-quantum-information/entanglement-in-action/qiskit-implementation
imports both
piandrandomfrom numpy. The notebook uses nothing else from numpy. These two objects are used only in a scalar context. In some cases, using numpy can degrade performance. In qiskit there has been an effort, reflected in PRs, to avoid using numpy habitually, when not necessary. This is done even in non-performance critical code.This PR changes the code to import
pifrommath. And it importsrandomfrom the stdlib. When producing single samples, standard libraryrandom.randintis about 5 times faster thannumpy.random.randint.Furthermore, the existing code calls numpy's
random.randint(0, 1), which always returns zero. This was mistakenly introduced. Since this PR usesrandintfrom the standard library, the range is 0 to 1, inclusive. So it may return either 0 or 1. My best understanding is that this is the original intent of the code.The following issues and PRs are on exactly the same code touched here
randomis imported from an inconsistent source #4643Closes #4643