virtual padding for invalid section_size in synth_cnot_count_full_pmh

[Abdalla01001]

Summary

Suggest using "virtual padding" or matrices filled with zeros to address empty and invalid circuits when the PMH algorithm cannot handle a section_size larger than the number of qubits.
#12106

Details

The issue in the code stems from how the algorithm handles the section_size parameter. The algorithm is designed to divide the input matrix into sections and process these sections iteratively. However, when the section_size is larger than the matrix size, there are no sections to iterate over, causing the algorithm to fail.

we can try to solve this by virtual padding, If the section_size exceeds the matrix size (n), we can virtually pad the matrix with additional columns of zeros to make it divisible by the section_size. This padding is "virtual" in the sense that it's not explicitly added to the matrix data structure, but rather accounted for in the algorithm's logic.

The changes:

• Virtual Padding: If the section_size is larger than the number of qubits (num_qubits), the matrix is padded with zeros to the right using np.hstack.

• Modified Loop: The loop in _lwr_cnot_synth now iterates up to state.shape[1] (the new padded size) to cover all sections.

• Handling Zero Sub-rows: The code now checks if a sub-row is all zeros (np.sum(sub_row_patt) == 0) and skips it if so.

• Removing Padding: When creating the circuit, we only add CNOT gates if both the control and target qubits are within the original num_qubits, effectively discarding operations on the padded columns.

• Modified Loop Range: The range of the inner loop in _lwr_cnot_synth is modified to range((sec - 1) * section_size, min(sec * section_size, num_qubits)). This ensures that the loop only iterates over the original columns, even if the section_size is larger than the matrix size.

[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

[CLAassistant]

All committers have signed the CLA.

[Abdalla01001]

The circuits correctness in old vs. new code:

from qiskit.quantum_info import Operator
from qiskit.circuit import QuantumCircuit
from qiskit.circuit.library import LinearFunction
from qiskit.synthesis import synth_cnot_count_full_pmh
from qiskit.synthesis.linear import random_invertible_binary_matrix

for nq in range(2, 10):
    bad_section_sizes = set()

    for seed in range(100, 200):
        mat = random_invertible_binary_matrix(nq, seed=seed)

        expected_qc = QuantumCircuit(nq)
        expected_qc.append(LinearFunction(mat), range(nq))
        expected_op = Operator(expected_qc)

        for section_size in range(1, nq+1):
            qc = synth_cnot_count_full_pmh(mat, section_size=section_size)
            op = Operator(qc)
            ok = op.equiv(expected_op)
            # print(f"section size is {section_size}: {ok}")
            if not ok:
                bad_section_sizes.add(section_size)

    print(f"For {nq = }: {bad_section_sizes = }")

without virtual padding:

For nq = 2: bad_section_sizes = set()
For nq = 3: bad_section_sizes = set()
For nq = 4: bad_section_sizes = set()
For nq = 5: bad_section_sizes = {3}
For nq = 6: bad_section_sizes = {4}
For nq = 7: bad_section_sizes = {4, 5}
For nq = 8: bad_section_sizes = {3, 5, 6}
For nq = 9: bad_section_sizes = {5, 6, 7}

with virtual padding:

For nq = 2: bad_section_sizes = set()
For nq = 3: bad_section_sizes = set()
For nq = 4: bad_section_sizes = set()
For nq = 5: bad_section_sizes = set()
For nq = 6: bad_section_sizes = set()
For nq = 7: bad_section_sizes = set()
For nq = 8: bad_section_sizes = set()
For nq = 9: bad_section_sizes = set()

[coveralls]

Pull Request Test Coverage Report for Build 9768925898

Details

• 15 of 15 (100.0%) changed or added relevant lines in 1 file are covered.
• 11 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.02%) to 89.807%

---

Files with Coverage Reduction	New Missed Lines	%
qiskit/synthesis/linear/cnot_synth.py	1	98.15%
crates/qasm2/src/lex.rs	4	92.88%
crates/qasm2/src/parse.rs	6	97.61%

Totals
Change from base Build 9767176201:	0.02%
Covered Lines:	64545
Relevant Lines:	71871

---

💛 - Coveralls

[ShellyGarion]

@Abdalla01001 - thanks for contributing to Qiskit and fixing the bug, but this PR needs some more work to fit into Qiskit standards.
I advise you to look at Qiskit contributing guidelines first.

1. Note that you should sign the CLA.

2. See the section on "Style and Lint".

3. See the section on "Release notes".

4. The functions API docs should not be removed. You can add a comment on virtual padding, but keep the existing format of the docstrings, since it appears here.

5. Please also add a test to this test file to check the correctness of the function with different values of section_size (which can be higher or smaller than the number of qubits).
   See for example this test:
   

   qiskit/test/python/synthesis/test_linear_synthesis.py

   Line 118 in bfc69a4

   def test_synth_lnn_kms(self, num_qubits):

[alexanderivrii]

could this be simplified to padding_size = section_size - num_qubits?

[Abdalla01001]

@alexanderivrii the goal of padding is to extend the matrix to a size divisible by the section_size. Just subtracting the number of qubits from the section size does not guarantee divisibility.

[ShellyGarion]

Following on this comment on the default value of section_size (which is currently 2).

@Cryoris noticed that in the paper they say one can choose section_size = alpha * log2(n) with alpha < 1 but the PRs only tested alpha = 1 maybe that's why the tests didn't perform any better? In the paper they do an experiment with alpha=1/2 (but say it may be suboptimal). Perhaps you can do some further tests with several values of alpha ?

See also this comment.

[coveralls]

Pull Request Test Coverage Report for Build 9774244854

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

• 13 of 13 (100.0%) changed or added relevant lines in 1 file are covered.
• 326 unchanged lines in 8 files lost coverage.
• Overall coverage increased (+0.07%) to 89.853%

---

Files with Coverage Reduction	New Missed Lines	%
qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py	4	96.92%
crates/qasm2/src/lex.rs	6	92.37%
qiskit/synthesis/one_qubit/one_qubit_decompose.py	14	70.67%
crates/circuit/src/dag_node.rs	27	85.37%
crates/accelerate/src/two_qubit_decompose.rs	42	89.5%
crates/accelerate/src/euler_one_qubit_decomposer.rs	46	93.34%
qiskit/dagcircuit/dagcircuit.py	61	92.14%
crates/circuit/src/operations.rs	126	81.23%

Totals
Change from base Build 9767176201:	0.07%
Covered Lines:	64794
Relevant Lines:	72111

---

💛 - Coveralls

[Cryoris]

After discussing with @alexanderivrii and @ShellyGarion we came to the conclusion that the section size being larger than the number of qubits is an invalid input. It would be better to raise an error if section_size > num_qubits, as originally done in #12166. Could you do that and remove the virtual padding?

We'll go forward with your PR then, as it still fixes the bad section_size values due to your fixes on L131 and L148.

[Abdalla01001]

That's right, I did it

[ShellyGarion]

Since the general plan is to port this code to Rust and we have a time pressure to make it to Qiskit 1.2 release, I'll close this PR is favor of #12588.
Both @Abdalla01001 and @Tarun-Kumar07 were added as co-authors of PR #12588.
We very much appreciate your suggestions and efforts in fixing this bug!