Separate State Preparation from Initialize #7666

javabster · 2022-02-15T22:39:32Z

Summary

Included in this PR:

State preparation code (minus the resets) has been moved from Initialize to a new StatePreparation class
Initialize now does the reset step and then appends a StatePreparation instruction
Static method get_num_qubits added to StatePreparation class
New tests added to check initialise decomposition to reset + stateprep

Notes

This PR could provide a solution to this SE query: https://quantumcomputing.stackexchange.com/questions/24535/is-it-possible-to-create-controlled-instructions-in-qiskit

coveralls · 2022-02-15T23:27:48Z

Pull Request Test Coverage Report for Build 2042467059

196 of 205 (95.61%) changed or added relevant lines in 4 files are covered.
3 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.006%) to 83.719%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/extensions/quantum_initializer/initializer.py	14	15	93.33%
qiskit/circuit/library/data_preparation/state_preparation.py	179	187	95.72%

Files with Coverage Reduction	New Missed Lines	%
qiskit/pulse/library/waveform.py	3	89.36%

Totals
Change from base Build 2042159833:	0.006%
Covered Lines:	53014
Relevant Lines:	63324

💛 - Coveralls

qiskit/extensions/quantum_initializer/initializer.py

jakelishman · 2022-02-16T10:39:19Z

qiskit/extensions/quantum_initializer/initializer.py

-        """
-        # call to generate the circuit that takes the desired vector to zero
-        disentangling_circuit = self.gates_to_uncompute()
+        self._stateprep = StatePreparation(params, num_qubits)


This stores the params inside another object. How well (relative to the current Initialize) will this handle a user mutating Instruction.params from under us once the instruction has been initialised?

can you give me a more specific example? I didn't explicitly check this but the params handling should just be the same if I moved everything over correctly

I didn't do any testing, but I was thinking that I could potentially do something pathological like

from qiskit.extensions import Initialize import numpy as np init = Initialize(np.array([1, 1]) * np.sqrt(0.5)) init.params = np.array([1, -1]) * np.sqrt(0.5)

and bad things would probably happen - in the new form, the params won't propagate through to StatePreparation. That said, it's highly likely that Initialize is already broken with respect to mutation of params in some form or another, and if so, it's not really your responsibility to fix it.

Running this example didn't throw any errors for me so either nothing bad is happening or bad things are happening silently 🤷‍♀️

I would assume this just does nothing, which is probably not the right behavior. I think there are two solutions to this: (1) either we add a params setter/getter which forwards the values to self._stateprep or (2) only create the StatePreparation within _define.

(1) couples Initialize and StatePrep very strongly but they are also almost the same so maybe that's the easier solution.

It wouldn't throw an error just on the code I wrote, but I was thinking about things that depend on params. Things like how init.definition behaves - both before and after this change, will it do the right thing if the parameters are mutated between object creation and first definition? My guess is that in the original code it will, and the new code it won't.

ah ok I understand what you mean now. I just checked with mutating params and the Initialize instruction params get updated but the StatePreparation doesn't.

I think I prefer option 1 from @Cryoris as we need to call self._stateprep.num_qubits to get the constructor to work properly in Initialize

The setter/getter is the best way to handle things at the moment, probably. It's still horribly broken (if I did init.params[2] = -init.params[2], it'd exhibit the same behaviour as it does now, even with a setter), but that's a problem that's fundamental to the way we treat params across the board in Terra, and which we hope to improve in the push for new classical instructions.

qiskit/extensions/quantum_initializer/initializer.py

jakelishman · 2022-02-16T11:11:54Z

qiskit/extensions/quantum_initializer/state_preparation.py

+        qubits (QuantumRegister or int):
+            * QuantumRegister: A list of qubits to be initialized [Default: None].
+            * int: Index of qubit to be initialized [Default: None].


What happens if it's None? The type hint is also inaccurate, since None is a valid input.

I'm not sure how best to document this. If the number of qubits in the circ matches exactly the number needed to append the initialize instruction then the user doesn't have to pass anything for this arg. But if the circ is longer they do need to pass this arg to specify which qubits to append the instruction to.

I think some form of the explanation you wrote is fine. Like

If given, a sequence of qubits or qubit specifiers to perform the initialisation on. If not given or None, then the initialisation will be applied to all the qubits in the circuit.

jakelishman · 2022-02-16T11:13:01Z

qiskit/extensions/quantum_initializer/state_preparation.py

+def prepare_state(self, state, qubits=None):
+    r"""Prpare qubits in a specific state.


This docstring (and similar for Initialize) could probably do with referencing each other, and explaining the difference between them.

jakelishman · 2022-02-16T11:14:48Z

qiskit/extensions/quantum_initializer/state_preparation.py

+class StatePreparation(Instruction):
+    """Complex amplitude state preparation.
+
+    Class that implements the (complex amplitude) state preparation of some
+    flexible collection of qubit registers.
+    """
+
+    def __init__(self, params, num_qubits=None):
+        """Prepare state


This class should at least explain that it's only correct state prep from an initial state of |00...0>, but it might even be better (and true to the original issue) if it accepted two possible arguments: start_state and end_state, where start_state defaults to |00...0>.

Maybe we could leave specifying a start_state for a different PR since that's a new feature and this merely splits Initialize into two 😄

But this definitely should make clear that it only works if you start from |0...>

I'm happy to leave start_state for another PR.

For Abby (via slack): the idea is that the current algorithm implements a gate sequence that takes you from |00...0> to the state you specified, so it's only correct state prep if the relevant registers start off in |00...0>. That wasn't an issue when it wasn't split, because Initialize called reset on the qubits, so it always started in |0>.

It's quite easy to extend this class to work from an arbitrary starting state; you generate the |0> -> |\psi> sequence for both start_state and end_state, and then just apply the adjoint of the start_state sequence before the end_state sequence, so you prep |initial> -> |0> -> |final>. It's not necessarily especially efficient, but the transpiler may well synthesis it into something better later.

jakelishman · 2022-02-16T11:16:01Z

test/python/circuit/test_initializer.py

+    def test_decompose_with_int(self):
+        """Test initialize with int arg decomposes to a StatePreparation and reset"""
+        qc = QuantumCircuit(2)
+        qc.initialize(2)
+        decom_circ = qc.decompose()
+        decom_circ2 = decom_circ.decompose()
+        dag1 = circuit_to_dag(decom_circ)
+        dag2 = circuit_to_dag(decom_circ2)
+
+        self.assertEqual(len(dag1.op_nodes()), 3)
+        self.assertEqual(dag1.op_nodes()[0].name, "reset")
+        self.assertEqual(dag1.op_nodes()[1].name, "reset")
+        self.assertEqual(dag1.op_nodes()[2].name, "state_preparation")
+
+        self.assertEqual(len(dag2.op_nodes()), 3)
+        self.assertEqual(dag2.op_nodes()[0].name, "reset")
+        self.assertEqual(dag2.op_nodes()[1].name, "reset")
+        self.assertEqual(dag2.op_nodes()[2].name, "x")


It wouldn't hurt to have at least one test that StatePreparation does essentially the same thing, but without the initial Reset.

jakelishman · 2022-02-16T11:16:42Z

test/python/circuit/test_initializer.py

+        dag1 = circuit_to_dag(decom_circ)
+        dag2 = circuit_to_dag(decom_circ2)
+
+        self.assertEqual(len(dag1.op_nodes()), 3)
+        self.assertEqual(dag1.op_nodes()[0].name, "reset")
+        self.assertEqual(dag1.op_nodes()[1].name, "reset")
+        self.assertEqual(dag1.op_nodes()[2].name, "state_preparation")


Is it necessary to convert to a DAG here? Can you compare based on QuantumCircuit.data?

yeah i think it should work comparing qc.data, is there a particular reason not to do the check by converting to a dag? I was mainly just following the style we use for other decompose tests

The only reason I mentioned it was because it looked odd to me, and I didn't immediately know the iteration order of DAGCircuit.op_nodes - there's no required total ordering in a DAG (only a partial ordering, but you can make a total order by sequencing the wires, I think).

qiskit/extensions/quantum_initializer/initializer.py

qiskit/extensions/quantum_initializer/state_preparation.py

Cryoris · 2022-02-18T09:29:12Z

qiskit/extensions/quantum_initializer/state_preparation.py

+class StatePreparation(Instruction):
+    """Complex amplitude state preparation.
+
+    Class that implements the (complex amplitude) state preparation of some
+    flexible collection of qubit registers.
+    """
+
+    def __init__(self, params, num_qubits=None):
+        """Prepare state


Maybe we could leave specifying a start_state for a different PR since that's a new feature and this merely splits Initialize into two 😄

But this definitely should make clear that it only works if you start from |0...>

qiskit/extensions/quantum_initializer/state_preparation.py

test/python/circuit/test_initializer.py

Cryoris · 2022-02-18T09:37:26Z

test/python/circuit/test_state_preparation.py

+        actual_sv = Statevector.from_instruction(qc)
+        self.assertTrue(desired_sv == actual_sv)
+
+    def test_decompose_with_int(self):


I don't think these tests are necessary, they seem to check that this operation is not the same as Initialize which is a very specific thing. Instead it might be better to test properties specific to StatePreparation, e.g. if the initial state of the qubits is not 0, the final state is not correct 🙂

Or what happens if you call qc.stateprep where the qubits arguments and state argument are incompatible?

javabster · 2022-02-22T23:12:14Z

docs are looking better now thanks @jakelishman 🎉

Cryoris

One last round of comments, then I think this is good to go!

qiskit/circuit/library/data_preparation/state_preparation.py

Cryoris · 2022-03-22T16:36:23Z

qiskit/circuit/library/data_preparation/state_preparation.py

+    ):
+        r"""
+        Args:
+            params (str, list, int or Statevector):


Could you remove the type hints from the docstring, now that they are in the signature? 🙂

qiskit/circuit/library/data_preparation/state_preparation.py

Co-authored-by: Julien Gacon <gaconju@gmail.com>

qiskit/circuit/library/data_preparation/state_preparation.py

Co-authored-by: Julien Gacon <gaconju@gmail.com>

jakelishman

I flicked through quickly and it looked fine to me. If Julien's happy with the changes since I last looked, this can be merged.

jakelishman · 2022-03-25T13:10:36Z

qiskit/circuit/library/data_preparation/state_preparation.py

+        self._params_arg = params
+        self._inverse = inverse
+        self._name = "state_preparation_dg" if self._inverse else "state_preparation"
+        self._label = f"{label} Dg" if self._inverse else label


This will cause StatePreparation().inverse().inverse() to have a label "StatePreparation Dg Dg" if you care about that sort of thing (I probably wouldn't bother).

Cryoris

We're missing a reno and one test case, then we can merge from my side! The test case would be checking that the params as integer raises correctly if the number of qubits is (1) missing or (2) too small.

…terra into stateprep-6246

Cryoris

LGTM, thanks for all the effort!

Abby-Mitchell and others added 3 commits January 17, 2022 17:59

Split initialize and stateprep first attempt

389493c

Tidied up code and added some tests

b4bc470

✨ lint ✨

ce41772

javabster requested a review from a team as a code owner February 15, 2022 22:39

javabster and others added 5 commits February 15, 2022 17:41

Tidied up code

47e21c8

Removed unnecessary file

bd32144

Merge branch 'main' into stateprep-6246

38c1b7a

Formatted docstrings

20c2fc3

Formatted docstrings again

18d25f4

jakelishman reviewed Feb 16, 2022

View reviewed changes

javabster added 3 commits February 16, 2022 10:01

✨ lint again ✨

9caa0e2

Fixed some docs bugs and added tests

bff8d76

Update docs and lint

34a1e68

Cryoris reviewed Feb 18, 2022

View reviewed changes

qiskit/extensions/quantum_initializer/initializer.py Outdated Show resolved Hide resolved

Cryoris reviewed Feb 18, 2022

View reviewed changes

qiskit/extensions/quantum_initializer/initializer.py Outdated Show resolved Hide resolved

Cryoris reviewed Feb 18, 2022

View reviewed changes

javabster added 3 commits February 18, 2022 12:20

docs fix

06d059b

Change StatePrep to a Gate and update tests

8c96081

Fixed tests and docs

b33a029

javabster and others added 9 commits February 22, 2022 20:18

✨ lint ✨

67535ee

Fixed test

7f06d8d

Add parmas getter and setter

f4fd20f

Fix stateprep inverse

46a910e

Merge branch 'main' into stateprep-6246

62b279e

Fix merge issues

5e5d469

Merge branch 'main' into stateprep-6246

e16f30e

Fix stateprep inverse

0a806c8

Merge branch 'main' into stateprep-6246

e94505c

Cryoris reviewed Mar 22, 2022

View reviewed changes

javabster and others added 5 commits March 23, 2022 09:19

Update qiskit/circuit/library/data_preparation/state_preparation.py

bf3a7b3

Co-authored-by: Julien Gacon <gaconju@gmail.com>

Update qiskit/circuit/library/data_preparation/state_preparation.py

9fa5d33

Co-authored-by: Julien Gacon <gaconju@gmail.com>

Update qiskit/circuit/library/data_preparation/state_preparation.py

df9f263

Co-authored-by: Julien Gacon <gaconju@gmail.com>

Merge branch 'main' into stateprep-6246

4dab2a6

Fixed docstrings and tests

9957485

Cryoris reviewed Mar 24, 2022

View reviewed changes

qiskit/circuit/library/data_preparation/state_preparation.py Outdated Show resolved Hide resolved

javabster and others added 4 commits March 24, 2022 13:29

Update qiskit/circuit/library/data_preparation/state_preparation.py

6e89658

Co-authored-by: Julien Gacon <gaconju@gmail.com>

Added repeat test

e61a05d

Merge branch 'main' into stateprep-6246

f9b4d03

Merge branch 'main' into stateprep-6246

b856ded

jakelishman previously approved these changes Mar 25, 2022

View reviewed changes

Cryoris requested changes Mar 25, 2022

View reviewed changes

javabster added 2 commits March 25, 2022 12:29

Added test and reno

42f0e29

Merge branch 'stateprep-6246' of https://github.com/javabster/qiskit-…

f87f555

…terra into stateprep-6246

javabster dismissed jakelishman’s stale review via f87f555 March 25, 2022 16:31

javabster added 2 commits March 25, 2022 12:48

Fix label issue

cd65b5c

Fixed docstrings

0dd97b8

Cryoris previously approved these changes Mar 25, 2022

View reviewed changes

✨ Lint! ✨

08e70d6

javabster dismissed Cryoris’s stale review via 08e70d6 March 25, 2022 19:03

Merge branch 'main' into stateprep-6246

681fdb6

jakelishman approved these changes Mar 25, 2022

View reviewed changes

jakelishman added Changelog: New Feature Include in the "Added" section of the changelog automerge labels Mar 25, 2022

Merge branch 'main' into stateprep-6246

e4735d3

mergify bot merged commit c318725 into Qiskit:main Mar 25, 2022

prakharb10 mentioned this pull request Jun 17, 2022

Fix documentation referring to StatePreparation #8194

Open

prakharb10 mentioned this pull request Jul 14, 2022

Fixed issue 4526 by adding InitializeGate #5179

Closed

ShellyGarion mentioned this pull request Apr 4, 2024

Use Isometry to implement Initialize instruction #6760

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate State Preparation from Initialize #7666

Separate State Preparation from Initialize #7666

javabster commented Feb 15, 2022 •

edited

Loading

coveralls commented Feb 15, 2022 •

edited

Loading

jakelishman Feb 16, 2022

javabster Feb 16, 2022

jakelishman Feb 16, 2022

javabster Feb 16, 2022

Cryoris Feb 18, 2022

jakelishman Feb 18, 2022

javabster Feb 18, 2022

jakelishman Feb 18, 2022

jakelishman Feb 16, 2022

javabster Feb 17, 2022

jakelishman Feb 18, 2022

jakelishman Feb 16, 2022

jakelishman Feb 16, 2022

Cryoris Feb 18, 2022

jakelishman Feb 18, 2022

jakelishman Feb 16, 2022

jakelishman Feb 16, 2022 •

edited

Loading

javabster Feb 16, 2022

jakelishman Feb 16, 2022

Cryoris Feb 18, 2022

Cryoris Feb 18, 2022

javabster commented Feb 22, 2022

Cryoris left a comment

Cryoris Mar 22, 2022

jakelishman left a comment

jakelishman Mar 25, 2022

Cryoris left a comment

Cryoris left a comment

		def prepare_state(self, state, qubits=None):
		r"""Prpare qubits in a specific state.

Separate State Preparation from Initialize #7666

Separate State Preparation from Initialize #7666

Conversation

javabster commented Feb 15, 2022 • edited Loading

Summary

Notes

coveralls commented Feb 15, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2042467059

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakelishman Feb 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javabster commented Feb 22, 2022

Cryoris left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakelishman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cryoris left a comment

Choose a reason for hiding this comment

Cryoris left a comment

Choose a reason for hiding this comment

javabster commented Feb 15, 2022 •

edited

Loading

coveralls commented Feb 15, 2022 •

edited

Loading

jakelishman Feb 16, 2022 •

edited

Loading