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

Replace initialization method by Isometry in StatePreparation #12178

Merged
merged 10 commits into from
May 8, 2024

Conversation

ShellyGarion
Copy link
Member

Summary

Close #12081
Replace initialization method by Isometry in StatePreparation

Details and comments

@ShellyGarion ShellyGarion added performance mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library labels Apr 14, 2024
@ShellyGarion ShellyGarion added this to the 1.1.0 milestone Apr 14, 2024
@ShellyGarion ShellyGarion requested a review from a team as a code owner April 14, 2024 11:47
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Cryoris
  • @Qiskit/terra-core
  • @ajavadia

@ShellyGarion ShellyGarion modified the milestones: 1.1.0, 1.2.0 Apr 18, 2024
@coveralls
Copy link

coveralls commented May 7, 2024

Pull Request Test Coverage Report for Build 8998409586

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.

Details

  • 21 of 21 (100.0%) changed or added relevant lines in 5 files are covered.
  • 1766 unchanged lines in 119 files lost coverage.
  • Overall coverage increased (+0.3%) to 89.631%

Files with Coverage Reduction New Missed Lines %
qiskit/converters/circuit_to_gate.py 1 97.44%
qiskit/circuit/classical/expr/constructors.py 1 91.82%
qiskit/circuit/library/standard_gates/r.py 1 97.62%
qiskit/circuit/controlflow/control_flow.py 1 92.86%
qiskit/transpiler/passes/layout/apply_layout.py 1 98.25%
qiskit/transpiler/target.py 1 93.74%
qiskit/primitives/statevector_sampler.py 1 99.08%
crates/qasm2/src/expr.rs 1 94.03%
qiskit/primitives/backend_sampler_v2.py 1 99.19%
qiskit/circuit/library/generalized_gates/permutation.py 1 93.1%
Totals Coverage Status
Change from base Build 8668339202: 0.3%
Covered Lines: 62195
Relevant Lines: 69390

💛 - Coveralls

@alexanderivrii
Copy link
Contributor

Thanks @ShellyGarion, this looks good. Hopefully all the tests will pass now. Could you please add the release note? And also some data on the number of gates / depth / etc. comparing the old and the new methods, i.e. highlighting improvement in this PR?

@ShellyGarion
Copy link
Member Author

ShellyGarion commented May 7, 2024

The following code compares the number of CX gates and circuit depth of this branch and the main branch:

    def test_cx_num(self):
        for n_qubits in [5, 7, 10, 12, 15]:
            state = np.random.rand(2 ** n_qubits) + np.random.rand(2 ** n_qubits) * 1j
            state = state / np.linalg.norm(state)

            # State preparation
            qc = QuantumCircuit(n_qubits)
            qc.append(StatePreparation(state), range(n_qubits))

            from qiskit import transpile
            transpiled = transpile(qc, basis_gates=['u', 'cx'], optimization_level=0)
            cx_sp = transpiled.count_ops()['cx']

            print("num qubits=", n_qubits, "cx count StatePreparation=", cx_sp, "depth StatePreparation=", transpiled.depth())

this branch:

num qubits= 5 cx count StatePreparation= 26 depth StatePreparation= 53
num qubits= 7 cx count StatePreparation= 120 depth StatePreparation= 241
num qubits= 10 cx count StatePreparation= 1013 depth StatePreparation= 2027
num qubits= 12 cx count StatePreparation= 4083 depth StatePreparation= 8167
num qubits= 15 cx count StatePreparation= 32752 depth StatePreparation= 65505

main branch:

num qubits= 5 cx count StatePreparation= 52 depth StatePreparation= 110
num qubits= 7 cx count StatePreparation= 240 depth StatePreparation= 488
num qubits= 10 cx count StatePreparation= 2026 depth StatePreparation= 4063
num qubits= 12 cx count StatePreparation= 8166 depth StatePreparation= 16345
num qubits= 15 cx count StatePreparation= 65504 depth StatePreparation= 131024

@alexanderivrii
Copy link
Contributor

Very nice, we see 2x reduction in the number of gates. Not directly related to this PR, but do you think it is worthwhile to track state preparation benchmarks via ASV (in the case that they are not there already)?

@ShellyGarion
Copy link
Member Author

Very nice, we see 2x reduction in the number of gates. Not directly related to this PR, but do you think it is worthwhile to track state preparation benchmarks via ASV (in the case that they are not there already)?

I think it would be nice to add it. I think that Isometry is being tracked in: https://github.com/Qiskit/qiskit/blob/main/test/benchmarks/isometry.py, but it would be nice to add a specific case for StatePreparation.

@ShellyGarion ShellyGarion changed the title [WIP] Replace initialization method by Isometry in StatePreparation Replace initialization method by Isometry in StatePreparation May 7, 2024
@ShellyGarion ShellyGarion added the Changelog: New Feature Include in the "Added" section of the changelog label May 7, 2024
Copy link
Contributor

@alexanderivrii alexanderivrii left a comment

Choose a reason for hiding this comment

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

LGTM! Just one small comment: I am personally in favor of removing internal code that is not likely to be ever used in the future. @ShellyGarion, you don't think that any of the removed methods in statepreparation.py will ever be useful, correct?

qiskit/circuit/library/generalized_gates/isometry.py Outdated Show resolved Hide resolved
@alexanderivrii
Copy link
Contributor

A quick run of ASV on the new statepreparation benchmark:

t have improved:

| Change   |   Before [b5c5179a] <main> |   After [4dab85c4] <pr-shelly> |   Ratio | Benchmark (Parameter)                                                                                   |
|----------|----------------------------|--------------------------------|---------|---------------------------------------------------------------------------------------------------------|
| -        |                         28 |                             17 |    0.61 | statepreparation.StatePreparationTranspileBench.track_cnot_counts_after_mapping_to_ibmq_16_melbourne(4) |
| -        |                        186 |                            111 |    0.6  | statepreparation.StatePreparationTranspileBench.track_cnot_counts_after_mapping_to_ibmq_16_melbourne(6) |
| -        |                         79 |                             47 |    0.59 | statepreparation.StatePreparationTranspileBench.track_cnot_counts_after_mapping_to_ibmq_16_melbourne(5) |
| -        |                        411 |                            231 |    0.56 | statepreparation.StatePreparationTranspileBench.track_cnot_counts_after_mapping_to_ibmq_16_melbourne(7) |
| -        |                        872 |                            460 |    0.53 | statepreparation.StatePreparationTranspileBench.track_cnot_counts_after_mapping_to_ibmq_16_melbourne(8) |

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.


alexanderivrii
alexanderivrii previously approved these changes May 8, 2024
@alexanderivrii alexanderivrii added this pull request to the merge queue May 8, 2024
Merged via the queue into Qiskit:main with commit 8b94fc3 May 8, 2024
15 checks passed
ElePT pushed a commit to ElePT/qiskit that referenced this pull request May 31, 2024
…#12178)

* replace initializetion method by Isometry in StatePreparation

* add names to QuantumRegister and QuantumCircuit

* update docs to the new reference

* remove old initialization code based on Shende et al

* fix reference in docs

* add release notes

* fix lint errors

* fix references in docs

* add a benchmark for state preparation

* update circuit name following review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More efficient synthesis of StatePreparation and Initialize based on Isometry
4 participants