C2QA / bosonic-qiskit

Extension of Qiskit to support hybrid boson-qubit simulations for the NQI C2QA effort.
BSD 2-Clause "Simplified" License
44 stars 8 forks source link

cv_initialize doesn't work well with multiple QumodeRegisters #84

Closed liu-zixiong closed 1 year ago

liu-zixiong commented 1 year ago

When creating multiple qumode registers, if the qumodes in all registers are not the same size, there will be errors when cv_initialize attempts to initialize a fock state on the qumodes.

Here is a circuit demonstrating the issue.

import c2qa

qmr1 = c2qa.QumodeRegister(1, 2)
qmr2 = c2qa.QumodeRegister(2, 2)
qmr3 = c2qa.QumodeRegister(2, 3) # <----- change to (2, 2) for no error
qmr4 = c2qa.QumodeRegister(1, 2)
qmr5 = c2qa.QumodeRegister(3, 2)
qmr6 = c2qa.QumodeRegister(3, 2)
circuit = c2qa.CVCircuit(qmr1, qmr2, qmr3, qmr4, qmr5, qmr6)

circuit.cv_initialize([0, 1], qmr1[0])
circuit.cv_initialize([0, 1], qmr2[0])
circuit.cv_initialize([0, 1], qmr3[0])
circuit.cv_initialize([0, 1], qmr4[0])
circuit.cv_initialize([0, 1], qmr5[0])
circuit.cv_initialize([0, 1], qmr6[0])

---------------------------------------------------------------------------
QiskitError                               Traceback (most recent call last)
[/Users/username/bosonic-qiskit/jupyter_clean.ipynb] Cell 6 in 1
     [14] circuit.cv_initialize([0, 1], qmr1[0])
     [15] circuit.cv_initialize([0, 1], qmr2[0])
---> [16] circuit.cv_initialize([0, 1], qmr3[0])
     [17] circuit.cv_initialize([0, 1], qmr4[0])
     [18] circuit.cv_initialize([0, 1], qmr5[0])

File [~/bosonic-qiskit/c2qa/circuit.py:260], in CVCircuit.cv_initialize(self, params, qumodes)
    257 for ind in range(len(params)):
    258     amplitudes[ind] = complex(params[ind])
--> 260 super().initialize(amplitudes, qumode)

File [/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/qiskit/extensions/quantum_initializer/initializer.py:191], in initialize(self, params, qubits)
    188     qubits = [qubits]
    189 num_qubits = len(qubits) if isinstance(params, int) else None
--> 191 return self.append(Initialize(params, num_qubits), qubits)

File [/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/qiskit/circuit/quantumcircuit.py:1298], in QuantumCircuit.append(self, instruction, qargs, cargs)
   1296 instructions = InstructionSet(resource_requester=requester)
   1297 if isinstance(operation, Instruction):
-> 1298     for qarg, carg in operation.broadcast_arguments(expanded_qargs, expanded_cargs):
   1299         self._check_dups(qarg)
   1300         instruction = CircuitInstruction(operation, qarg, carg)

File [/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/qiskit/circuit/library/data_preparation/state_preparation.py:220], in StatePreparation.broadcast_arguments(self, qargs, cargs)
    217 flat_qargs = [qarg for sublist in qargs for qarg in sublist]
    219 if self.num_qubits != len(flat_qargs):
--> 220     raise QiskitError(
    221         "StatePreparation parameter vector has %d elements, therefore expects %s "
    222         "qubits. However, %s were provided."
    223         % (2**self.num_qubits, self.num_qubits, len(flat_qargs))
    224     )
    225 yield flat_qargs, []

QiskitError: 'StatePreparation parameter vector has 4 elements, therefore expects 2 qubits. However, 3 were provided.'

Note that the traceback changes depending on which qumode register object holds the qumode of a different size. For the case above, if num_qubits_per_qumode = 3 for qmr3, error occurs when initializing qmr3. But if I feed num_qubits_per_qumode = 3 into qmr6, I actually get an error when initializing qmr1.

import c2qa
...
...
qmr5 = c2qa.QumodeRegister(3, 2)
qmr6 = c2qa.QumodeRegister(3, 3)
circuit = c2qa.CVCircuit(qmr1, qmr2, qmr3, qmr4, qmr5, qmr6)
...
...

---------------------------------------------------------------------------
QiskitError                               Traceback (most recent call last)
[/Users/username/bosonic-qiskit/jupyter_clean.ipynb] Cell 6 in 1
     [11] qmr6 = c2qa.QumodeRegister(3, 3)
     [12] circuit = c2qa.CVCircuit(qmr1, qmr2, qmr3, qmr4, qmr5, qmr6)
---> [14] circuit.cv_initialize([0, 1], qmr1[0])
     [15] circuit.cv_initialize([0, 1], qmr2[0])
     [16] circuit.cv_initialize([0, 1], qmr3[0])

File [~/bosonic-qiskit/c2qa/circuit.py:260], in CVCircuit.cv_initialize(self, params, qumodes)
    257 for ind in range(len(params)):
    258     amplitudes[ind] = complex(params[ind])
--> 260 super().initialize(amplitudes, qumode)

File [/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/qiskit/extensions/quantum_initializer/initializer.py:191], in initialize(self, params, qubits)
    188     qubits = [qubits]
    189 num_qubits = len(qubits) if isinstance(params, int) else None
--> 191 return self.append(Initialize(params, num_qubits), qubits)

File [/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/qiskit/circuit/quantumcircuit.py:1298], in QuantumCircuit.append(self, instruction, qargs, cargs)
   1296 instructions = InstructionSet(resource_requester=requester)
   1297 if isinstance(operation, Instruction):
-> 1298     for qarg, carg in operation.broadcast_arguments(expanded_qargs, expanded_cargs):
   1299         self._check_dups(qarg)
   1300         instruction = CircuitInstruction(operation, qarg, carg)

File [/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/qiskit/circuit/library/data_preparation/state_preparation.py:220], in StatePreparation.broadcast_arguments(self, qargs, cargs)
    217 flat_qargs = [qarg for sublist in qargs for qarg in sublist]
    219 if self.num_qubits != len(flat_qargs):
--> 220     raise QiskitError(
    221         "StatePreparation parameter vector has %d elements, therefore expects %s "
    222         "qubits. However, %s were provided."
    223         % (2**self.num_qubits, self.num_qubits, len(flat_qargs))
    224     )
    225 yield flat_qargs, []

QiskitError: 'StatePreparation parameter vector has 8 elements, therefore expects 3 qubits. However, 2 were provided.'
tjstavenger-pnnl commented 1 year ago

I think this is an issue @kevincsmith and I spoke about last week -- we should be sure to use the cutoff for the appropriate register. CVCircuit has one generic cutoff property that just returns the last register's cutoff. Any uses of this function should be modified to get the appropriate registers' cutoffs instead. See https://github.com/C2QA/bosonic-qiskit/blob/main/c2qa/circuit.py#L108-L111

tjstavenger-pnnl commented 1 year ago

We might be able to correct at least a part of this without having to redefine the CVCircuit.cutoff that also relates to #86.

The existing cv_initialize doesn't use the cutoff property function, but instead references the last qumode's cutoff directly instead. See:

I believe in all cases we could use the qumode index instead and lookup the actual cutoff vs using the [-1] last syntax. Lines 215 and 224 would need to be moved into the loops in order to check the param value against each qumode's cutoff.

Though if the qumodes is just a list of qubits (as it appears the the docs it can be), we'd still have to lookup which regster the qubit belongs to.

tjstavenger-pnnl commented 1 year ago

@liu-zixiong -- you'll find in branch cv-initialize-per-qumode and PR #89 your test code above "working" in that it doesn't throw an error. No real test validating data, though. We could compare the counts / state vector that comes from the simulation? I changed the cv_initialize to use each qumode's cutoff, but I would expect we may need to do the same across all of our gates, too.

liu-zixiong commented 1 year ago

Thanks for the update Tim! As far as I'm aware, the current repo does not have gates that work with separate qumode registers, so it might not be possible to produce validating data via simulate without first addressing #86.

Nonetheless, do you have plans to merge PR #89 already? We could close this report after merging since the details on qumode cutoff for all gates is already reported in issue #86.

tjstavenger-pnnl commented 1 year ago

I think it can be merged now, I'm comfortable with that. Especially if it can help us address #86 and then get more tests implemented.