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

QPY can't serialize UCRZGate #7847

Closed
HuangJunye opened this issue Mar 31, 2022 · 2 comments · Fixed by #7933
Closed

QPY can't serialize UCRZGate #7847

HuangJunye opened this issue Mar 31, 2022 · 2 comments · Fixed by #7933
Assignees
Labels
bug Something isn't working mod: qpy Related to QPY serialization

Comments

@HuangJunye
Copy link
Collaborator

HuangJunye commented Mar 31, 2022

Environment

  • Qiskit Terra version: 0.19.2
  • Python version: 3.9.7
  • Operating system: macOS Monterey 12.2.1

What is happening?

While developing a Grover's tutorial for the new Qiskit Runtime primitives, I found a bug where the grover circuit submitted to runtime provider with sampler program return TypeError: __init__() takes 2 positional arguments but 5 were given. After some investigation, I have narrowed down the issue to UCRZGate which is part of the Grover operator in the grover circuit.

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Input In [1], in <cell line: 20>()
     18 circuit_json = json.loads(json_str)
     19 qpy_file = io.BytesIO(base64.b64decode(circuit_json["circuits"]))
---> 20 circuit = qpy_serialization.load(qpy_file)[0]

File /usr/local/anaconda3/envs/primitives/lib/python3.9/site-packages/qiskit/circuit/qpy_serialization.py:1776, in load(file_obj)
   1774 circuits = []
   1775 for _ in range(file_header[5]):
-> 1776     circuits.append(_read_circuit(file_obj, qpy_version))
   1777 return circuits

File /usr/local/anaconda3/envs/primitives/lib/python3.9/site-packages/qiskit/circuit/qpy_serialization.py:1912, in _read_circuit(file_obj, version)
   1910 custom_instructions = _read_custom_instructions(file_obj, version, vectors)
   1911 for _instruction in range(num_instructions):
-> 1912     _read_instruction(file_obj, circ, out_registers, custom_instructions, version, vectors)
   1913 for vec_name, (vector, initialized_params) in vectors.items():
   1914     if len(initialized_params) != len(vector):

File /usr/local/anaconda3/envs/primitives/lib/python3.9/site-packages/qiskit/circuit/qpy_serialization.py:1121, in _read_instruction(file_obj, circuit, registers, custom_instructions, version, vectors)
   1119         elif gate_name in {"BreakLoopOp", "ContinueLoopOp"}:
   1120             params = [len(qargs), len(cargs)]
-> 1121         gate = gate_class(*params)
   1122     gate.condition = condition_tuple
   1123 if label_size > 0:

TypeError: __init__() takes 2 positional arguments but 5 were given

How can we reproduce the issue?

import io
import json
import base64
from numpy import pi

from qiskit.circuit import qpy_serialization
from qiskit import QuantumCircuit

qc = QuantumCircuit(3)
qc.ucrz([0,0,0,-pi], [1,2], [0])
qc.measure_all()

buf = io.BytesIO()
qpy_serialization.dump(qc, buf)
json_str = json.dumps({
    'circuits': base64.b64encode(buf.getvalue()).decode('utf8')
})
circuit_json = json.loads(json_str)
qpy_file = io.BytesIO(base64.b64decode(circuit_json["circuits"]))
circuit = qpy_serialization.load(qpy_file)[0]

qpy code based on this answer by @mtreinish on StackExchange.

What should happen?

Circuits containing UCRZGate should be able to be serialized by QPY.

Any suggestions?

No response

@HuangJunye HuangJunye added bug Something isn't working mod: qpy Related to QPY serialization labels Mar 31, 2022
@mtreinish
Copy link
Member

The problem is the UCRZGate is non-standard in it's construction. It looks like it sets the params attribute to the angle list (as does anything based on UCPauliRotGate. By default qpy deserialization tries to do deserialize a gate as cls(*params) to create an instance of the gate in the circuit. However that doesn't work because the ucrz params is the angle list. To fix this we'll probably need to do two things, first special case the deserialization for the UCPauliRotGate family of gates here:

https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/qpy/binary_io/circuits.py#L240-L257

so that qpy knows that it needs to handle the creation of them separately as they're not built in the standard way. Secondly it might require updating this function if we need to include extra attributes in the serialization of the gate https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/qpy/binary_io/circuits.py#L382 function to enable creating a new equivalent instance in deserialization (I'm thinking for a generic UCPauliRotGate we might need to embed the rotation axis as an extra parameter as that's not stored in the params list)

@HuangJunye
Copy link
Collaborator Author

I see. That sounds like my next project :D Can I assign myself on this issue? I will take a closer look at QPY later. For the purpose of creating the Grover tutorial, I am going to use decomposed circuits below UCRZ level which I have tested working.

@HuangJunye HuangJunye self-assigned this Mar 31, 2022
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Apr 13, 2022
This commit fixes the deserialization of the UCRX, UCRY, and UCRZ gates.
These gates aren't constructed in the standard way as other gate objects
where the parameters are passed directly as args to the class
constructor. Instead they behave the same as Initialize and the params
are passed as a list to the constructor to make a new gate object. This
commit fixes the handling in deserialization to treat these gates the
same as Initialize so that it can be properly deserialized by
qpy.load().

Fixes Qiskit#7847
@HuangJunye HuangJunye assigned mtreinish and unassigned HuangJunye Apr 14, 2022
@mergify mergify bot closed this as completed in #7933 Apr 14, 2022
mergify bot added a commit that referenced this issue Apr 14, 2022
This commit fixes the deserialization of the UCRX, UCRY, and UCRZ gates.
These gates aren't constructed in the standard way as other gate objects
where the parameters are passed directly as args to the class
constructor. Instead they behave the same as Initialize and the params
are passed as a list to the constructor to make a new gate object. This
commit fixes the handling in deserialization to treat these gates the
same as Initialize so that it can be properly deserialized by
qpy.load().

Fixes #7847

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mergify bot pushed a commit that referenced this issue Apr 14, 2022
This commit fixes the deserialization of the UCRX, UCRY, and UCRZ gates.
These gates aren't constructed in the standard way as other gate objects
where the parameters are passed directly as args to the class
constructor. Instead they behave the same as Initialize and the params
are passed as a list to the constructor to make a new gate object. This
commit fixes the handling in deserialization to treat these gates the
same as Initialize so that it can be properly deserialized by
qpy.load().

Fixes #7847

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 2f182f6)
mergify bot added a commit that referenced this issue Apr 14, 2022
This commit fixes the deserialization of the UCRX, UCRY, and UCRZ gates.
These gates aren't constructed in the standard way as other gate objects
where the parameters are passed directly as args to the class
constructor. Instead they behave the same as Initialize and the params
are passed as a list to the constructor to make a new gate object. This
commit fixes the handling in deserialization to treat these gates the
same as Initialize so that it can be properly deserialized by
qpy.load().

Fixes #7847

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 2f182f6)

Co-authored-by: Matthew Treinish <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working mod: qpy Related to QPY serialization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants