[unitaryHACK] Controlling the Insertion of Multi-Qubit Gates in the Generation of Random Circuits #12059

[shravanpatel30]

This pull request is submitted as a solution for issue #12059.

I have added a feature to the random_circuit() function that will allow users to specify a 'num_operand_distribution' by which thay can control the distribution of 1-qubit, 2-qubit, 3-qubit, and 4-qubit gates in the circuit. Alongwith the code I have also updated the release notes and have included some tests.

Thank you in advance for reviewing.

Summary

Details and comments

[qiskit-bot]

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:

• @Qiskit/terra-core

[1ucian0]

Hi @shravanpatel30, thanks for the PR! the code has some linting issues. I'm moving this one to draft. Once ready for review, you can move it out of draft.

[1ucian0]

In general, it is better not to change the order of the positional arguments. Otherwise, you might accidentally break callers like random_circuit(2, 4, 10)

[shravanpatel30]

Hi @1ucian0, thanks for reviewing the PR. I was able to fix the linting issues but I have a question about the testing. When I run tox I can see one failure when test_random_mid_circuit_measure_conditional function is checked. I think the problem arises because seed=4 yields a different random circuit than it used to for old code. I know how to fix that issue (by changing seed =4 to seed =16 to produce a similar circuit), should I make this change and include it in my PR or should I not change other tests?

[qiskit-bot]

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:

• @Qiskit/terra-core

[sbrandhsn]

Thanks for your contribution @shravanpatel30! :-) 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 -eblack and tox -elint-incr locally 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_operands and num_operand_distribution set is fine, please revert the initial default value of max_operands. If num_operand_distribution is not set but max_operands is set, draw a random num_operand_distribution and 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 depth as a parameter. This should make your num_operand_distribution match closely what is contained in the returned random circuit.

[sbrandhsn]

What does the constant 5 refer to here? Can you derive the value for that constant from all_gate_lists?

[shravanpatel30]

This constant 5 here refers to an array that will have total length len(all_gate_lists) + 1, so I can just derive this value from all_gate_lists.

[sbrandhsn]

Also, you can fixing that failing test case in the described way seems fine to me. :-)

[shravanpatel30]

Sorry for the multiple commits, but I am facing an issue with runnin tox - elint locally (I have opened an issue in qiskit regarding that).

@sbrandhsn I have addressed your comments in my recent commits. Here are some details about the changes:

• There are formatting issues which causes our CI to fail. Please run tox -eblack and tox -elint-incr locally to figure out what needs to be changed.

• I have fixed the linting and code formatting issues. The CI is able to run all the tests successfully.

• I think I made a mistake when specifying the precedence of max_operands, sorry! Having both set, max_operands and num_operand_distribution set is fine, please revert the initial default value of max_operands. If num_operand_distribution is not set but max_operands is set, draw a random num_operand_distribution and continue with your code or use the initial code.

• Now my code won't raise error when both num_operand_distribution and max_operands are set. And if both are set the code will use the distribution specified in num_operand_distribution. And the max_operands will use the default value 4 (according to the original 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.

• In my earlier code if {1: 0, ...} is specified explicitly then: 1) the distribution list will have 0 probability corresponding to the 1-qubit gates in gates_to_consider and so the np.choice(..., p=distribution) will never pick the 1-qubit gates. 2) To handle how the non-used qubits are filled, I have a slack handling loop, which will only add gates that have a non-zero probability in num_operand_distribution. I have extensively tested these two logics and have also written a test to confirm this. But if you still think this needs to be changed then let me know.

• Please add a test case with some distribution and specify a particularly long depth as a parameter. This should make your num_operand_distribution match closely what is contained in the returned random circuit.

• I have added three more tests (total 4) which will test if the code respects the num_operand_distribution or not by checking if there are any unwanted gates in the circuit or not.

• Also, you can fixing that failing test case in the described way seems fine to me. :-)

• I have fixed this test.

Question

• Please raise an error when max_operands is larger than num_qubits.

• I have commented this above where you originally asked this. But this is how the original code handled when max_operands is larger than num_qubits. Should I still raise an error when this happens? I am asking this because the using max_operands=4 (which was its default value in original code) as the default value, the code will always throw error for num_qubits<4 when user calls random_circuit(num_qubits=3, depth=3). And this is how most of the users call random_circuits() i.e. without ever specifying max_operands.

[sbrandhsn]

I did a quick plot to see how accurate the random circuit generation is and it looks pretty good. I was wondering whether you have an idea on why the ratios of 1-q and 4-q gates have a larger distance to the specified distribution than 2-q and 3-q gates. :-)

Code for the plot:

import numpy as np
import pandas as pd
from matplotlib import pyplot as plt
from qiskit.circuit.random import random_circuit

rng = np.random.default_rng()
iterations = 100
qubits = 20
depths = [1, 5, 10, 20, 50, 70, 100, 200, 300, 500, 700, 1000]
depths = np.linspace(1, 100, 50, dtype=int)
data_points = []
for _ in range(iterations):
    rnd_distr = {i+1: r for i, r in enumerate(rng.dirichlet(np.ones(4)))}
    print(rnd_distr)
    for d in depths:
        qc = random_circuit(qubits, d, num_operand_distribution=rnd_distr)
        distance_to_distribution = {}
        for n_qargs, prob in rnd_distr.items():
            n_nq_gates = qc.size(lambda x: not getattr(x.operation, "_directive", False) and len(x.qubits) == n_qargs)
            distance_to_distribution[n_qargs] = n_nq_gates/qc.size()

        distance_to_distribution['d'] = d
        for n_qargs, prob in rnd_distr.items():
            distance_to_distribution[n_qargs] = abs(distance_to_distribution[n_qargs]-prob)
        distance_to_distribution['l1'] = sum(distance_to_distribution[n_qargs] for n_qargs, prob in rnd_distr.items())
        data_points.append(distance_to_distribution)

df = pd.DataFrame.from_records(data_points)
df = df.groupby('d').agg('mean').reset_index()
for i in range(1, 5):
    plt.plot(df.d, df[i], label=f'{i}-q gates')

plt.plot(df.d, df.l1, label='Sum')
plt.xlabel('Circuit Depth')
plt.ylabel('L1-Distance')
plt.yscale('log')
plt.legend()
plt.show()

[shravanpatel30]

I did a quick plot to see how accurate the random circuit generation is and it looks pretty good. I was wondering whether you have an idea on why the ratios of 1-q and 4-q gates have a larger distance to the specified distribution than 2-q and 3-q gates. :-)

@sbrandhsn I'm not exactly sure why we are seeing this but here is what I think. The larger distance between 1-q and 4-q gates is because it is easier to fit in 1-q gates and harder to fit in 4-q gates using the current implementation. The function np.cumsum truncates the randomly drawn gate_specs array which will introduce some bias that will lead to the problem we are seeing. And due to this bias the no. of 1-q gates will be almost always be higher than expected and the no. of 4-q gates will be lower than expected. When I wasworking on this issue I did try to come up with a solution that does not involve truncating the randomly drawn gate_specs array but that will slow the code down quite a bit.

But if we look at the distribution of gates for a random circuit with 100 and 500 qubits, what I observed is that distance from the expected distribution decreases.

• qubits=100:
  

• qubits=500:

We will observe a similar trend if we just draw a random distribution from only 1-q, 2-q and 3-q gates (excluding 4-q gates). Below is the plot for that

Here we see the same issue but with 1-q and 3-q gates.

[sbrandhsn]

This LGTM, thanks! Can you add some information we obtained from the plots to the code? Something along the lines of:
'Since generating a random circuit involves stochastic processes, the provided ratios will not always be exactly matched. Expect higher deviations from the specified ratios for quantum circuits with a small number of qubits and lower depth and lower deviations for large quantum circuits.'
I was also thinking about giving specific numbers but this seems like too much. :-)

[shravanpatel30]

@sbrandhsn I have added the comment in the code and in that I have also mentioned that for more details on how the gate distribution changes with no. of qubits and depth refer to this pull request.

[sbrandhsn]

LGTM, thanks!

[sbrandhsn]

I hope you don't mind the small update to the argument documentation - please update how you see fit. :-) Thanks for your contribution, I will accept the PR!

[coveralls]

Pull Request Test Coverage Report for Build 9414985165

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 45 of 49 (91.84%) changed or added relevant lines in 1 file are covered.
• 69 unchanged lines in 4 files lost coverage.
• Overall coverage increased (+0.03%) to 89.585%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/random/utils.py	45	49	91.84%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	3	92.37%
crates/circuit/src/circuit_instruction.rs	18	89.09%
crates/circuit/src/circuit_data.rs	24	93.98%
qiskit/visualization/circuit/text.py	24	95.23%

Totals
Change from base Build 9407377778:	0.03%
Covered Lines:	62450
Relevant Lines:	69710

---

💛 - Coveralls

[sbrandhsn]

Please go ahead and add a comment to #12059 so I can assign the issue to you, thanks! :-)

[coveralls]

Pull Request Test Coverage Report for Build 9415220376

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 45 of 49 (91.84%) changed or added relevant lines in 1 file are covered.
• 75 unchanged lines in 5 files lost coverage.
• Overall coverage increased (+0.02%) to 89.581%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/random/utils.py	45	49	91.84%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	3	91.86%
crates/qasm2/src/parse.rs	6	97.61%
crates/circuit/src/circuit_instruction.rs	18	89.09%
crates/circuit/src/circuit_data.rs	24	93.98%
qiskit/visualization/circuit/text.py	24	95.23%

Totals
Change from base Build 9407377778:	0.02%
Covered Lines:	62447
Relevant Lines:	69710

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 9415861397

Details

• 45 of 49 (91.84%) changed or added relevant lines in 1 file are covered.
• 17 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.01%) to 89.571%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/random/utils.py	45	49	91.84%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	5	92.37%
crates/qasm2/src/parse.rs	12	96.69%

Totals
Change from base Build 9410059639:	-0.01%
Covered Lines:	62440
Relevant Lines:	69710

---

💛 - Coveralls