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

[WIP] Add phase property to Gate class #3472

Closed

Conversation

chriseclectic
Copy link
Member

@chriseclectic chriseclectic commented Nov 14, 2019

Summary

  • Adds phase and phase_angle properties to Gate class for storing global phasee
  • Updates non-controlled standard gates with phase (including definitions)
  • Adds matrix definitions to all non-controlled standard gates
  • Changes to_matrix to return the matrix definition times the phase
  • Adds matrix definition to gate doc strings for standard gates

Details and comments

When constructing a gate a phase can be set using phase=theta kwarg. theta will be stored as a float [-2*pi, 2*pi] in the gate object, representing a complex coefficient exp(i theta) on the matrix definition of the gate.

After construction a gate object can have its phase modified using gate.phase = theta.

When calling to_matrix the returned matrix will be exp(i theta) * U_def where U_def is the matrix definition of the gate.

When unrolling any set phase parameter will be passed to one of the gates in the definition so that the unrolled circuit has the same global phase as the original gate.

TODO

  • update controlled gates with phase
  • update 1-qubit synthesis to respect global phase
  • update 2-qubit synthesis to respect global phase
  • add tests
  • convert gate phases to a circuit global phase when appended to circuits (this might need to be a separate PR)

Fixes #3304

@ewinston
Copy link
Contributor

Why have phase and phase_angle? Seems like more to keep track of and check.

qiskit/circuit/gate.py Outdated Show resolved Hide resolved
qiskit/circuit/gate.py Outdated Show resolved Hide resolved
qiskit/circuit/gate.py Outdated Show resolved Hide resolved
qiskit/circuit/gate.py Outdated Show resolved Hide resolved
qiskit/circuit/gate.py Outdated Show resolved Hide resolved
@ajavadia
Copy link
Member

Why have phase and phase_angle? Seems like more to keep track of and check.

@ewinston it only has one internal attribute, the rest are getter/setter over that.

@ewinston
Copy link
Contributor

ewinston commented Dec 2, 2019

@ajavadia What if phase was what phase_angle currently is since it's shorter and I am not sure of a reason to support the rectangular form?

@chriseclectic
Copy link
Member Author

As @ewinston suggested (and several others also requested) removed the non-polar form of phase and renamed phase_angle to just phase

@chriseclectic
Copy link
Member Author

@ewinston I rebase to fix conflicts with master, but there are still some issues with controlled gates and failing tests

@@ -32,6 +32,10 @@ def add_control(operation, num_ctrl_qubits, label):
num_qubits + 2*num_ctrl_qubits - 1.
"""
import qiskit.extensions.standard as standard
if operation.phase:
Copy link
Member Author

Choose a reason for hiding this comment

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

I would like to be able to remove this clause and have the phase case handled by the standard control function without converting to a unitary first.

Change simplify atol to simplify_tolerance
* Added phase to base Gate class
* Added phase to base ControlledGate class
* Added phase to standard gate extensions
@chriseclectic
Copy link
Member Author

Made a new PR #3930 since only way to clean up history with master was huge squash rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RzGate(alpha).to_matrix() is missing
4 participants