add a new parameter num_qubits_distribution to random_circuit #12475
Conversation
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the following people are relevant to this code:
|
|
|
|
KoheiSuda seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Dear Kohei-san, thanks for showing interest in this issue. I will be on vacation and look into this PR next week! In the meantime make yourself familiar with https://github.com/Qiskit/qiskit/blob/main/CONTRIBUTING.md and the unitaryhack rules (if you have not already). :-) |
There was a problem hiding this comment.
Thanks for this contribution @KoheiSuda! :-) This looks like an elegant solution that needs a couple of modifications before it can be accepted.
- There are formatting issues which causes our CI to fail. Please run
tox -eblackandtox -elint-incrlocally to figure out what needs to be changed. - I think I made a mistake when specifying the precedence of
max_operands, sorry! Having both set,max_operandsandnum_operand_distributionset is fine, please revert the initial default value ofmax_operands. Ifnum_operand_distributionis not set butmax_operandsis set, draw a randomnum_operand_distributionand continue with your code or use the initial code. - Please add a test case with a distribution that disallows single-qubit gates, i.e. with {1:0, …}. I think some logic around how single-qubit gates are used to fill the non-used qubits in layer would fail such a test case with the current code.
- Please add a test case with some distribution and specify a particularly long
depthas a parameter. This should make yournum_operand_distributionmatch closely what is contained in the returned random circuit. - The CLAAssistant appears to have an issue with your e-mail address, can you fix that? :-)
|
@sbrandhsn
I have a question. If a distribution disallows single-qubit gates, how should I fill the non-used qubits in layer? Is it okay to use single-qubit gates? |
|
Dear @KoheiSuda, I'm sorry but a PR was submitted by a different user that addresses #12059 completely. Unfortunately, unitaryHACK explicitly allows multiple developers to work on the same issue at once ( https://unitaryhack.dev/rules/ ) and essentially requires us to select the first valid PR for the bounty. Qiskit still has open unitaryHACK issues available at https://github.com/Qiskit/qiskit/issues?q=is%3Aopen+label%3Aunitaryhack-bounty+is%3Aissue that (partially) attracted interest by other developers. Apart from this, I was very happy about your elegant contribution so far and can only encourage you to continue your good work on open source projects that you find interesting! :-) |
|
@sbrandhsn |
Summary
fixes #12059
This PR extends the random_circuit function in Qiskit to allow users to specify a distribution num_operand_distribution that defines the ratio of 1-qubit, 2-qubit, ..., n-qubit gates in the generated random circuit. For example, calling random_circuit with num_operand_distribution={1: 0.5, 2: 0.5} will produce random circuits with approximately 50% single-qubit gates and 50% two-qubit gates, while disallowing gates with more than 2 qubits.
Additionally, the following points have been addressed:
Details and comments
I worked on this issue as part of unitaryHACK, so please assign this issue to me if this PR is accepted.