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

Operator.power(n) only works for integer n #11454

Closed
alexanderivrii opened this issue Dec 21, 2023 · 7 comments · Fixed by #11534
Closed

Operator.power(n) only works for integer n #11454

alexanderivrii opened this issue Dec 21, 2023 · 7 comments · Fixed by #11534
Assignees
Labels
bug Something isn't working
Milestone

Comments

@alexanderivrii
Copy link
Contributor

alexanderivrii commented Dec 21, 2023

Environment

  • Qiskit Terra version: current
  • Python version: shouldn't matter
  • Operating system: shouldn't matter

What is happening?

In the description of the Operator.power(n) method:

def power(self, n: float) -> Operator:
"""Return the matrix power of the operator.
Args:
n (float): the power to raise the matrix to.
Returns:
Operator: the resulting operator ``O ** n``.
Raises:
QiskitError: if the input and output dimensions of the operator
are not equal.
"""

the argument power is of type float, however in practice this code only works for integer values of n.

How can we reproduce the issue?

import numpy as np
from qiskit.circuit.library import CXGate
from qiskit.quantum_info import Operator

op = Operator(CXGate())
print(op)
op1 = op.power(3)
print(op1)
op2 = op.power(3.14)
print(op2)

What should happen?

The power method in gate.py:

def power(self, exponent: float):
"""Creates a unitary gate as `gate^exponent`.
Args:
exponent (float): Gate^exponent
Returns:
.library.UnitaryGate: To which `to_matrix` is self.to_matrix^exponent.
Raises:
CircuitError: If Gate is not unitary
"""

does work with floating-point powers and (as far as I can judge) could be moved to the Operator class.

Any suggestions?

No response

@alexanderivrii alexanderivrii added the bug Something isn't working label Dec 21, 2023
@alexanderivrii alexanderivrii self-assigned this Dec 21, 2023
@alexanderivrii
Copy link
Contributor Author

I have assigned myself to the issue. However, any thoughts on
(1) whether supporting general floating-point powers for Operator class should or should not be allowed, and
(2) whether a one-line method to raise a complex-valued matrix to a floating point already exists (and which would probably be equivalent to the code in gate.py anyway)
are welcome.

@alexanderivrii alexanderivrii added this to the 1.0.0 milestone Dec 21, 2023
@sadbro
Copy link

sadbro commented Dec 29, 2023

The need of nth root of a operator may sound convincing if supporting general floating powers is in question
A one liner does exists in scipy => scipy.linalg.fractional_matrix_power(A, t) where A is a square matrix of order n and t is a fractional power, but a implementation can be made for more robustness

@ajavadia
Copy link
Member

Yes I think it can be supported. We just have to use the same code that is used in the Gate class now, which does in fact raise a matrix to a power using scipy.linalg.schur and the output type is currently a UnitaryGate. I prefer natively supporting an Operator in the circuit rather than UnitaryGate (which seems like a duplication of Operator from when we could not put Operator in the circuit). In that sense I would prefer the raising to a power to return an Operator, and the code to do this to reside in the Operator.

Also I think the "control" method should also work for operators. It amounts to increasing the dimension and making the new matrix a direct sum of Identity and U (if control is the lower qubit according to qiskit's little endian convention, otherwise a permutation of this matrix).

@jakelishman
Copy link
Member

Fractional-power Operator seems completely reasonable.

To speak to Ali's other point: I think in the higher-abstraction-level world QuantumCircuit lives in now, there is still a need for both Operator and UnitaryGate. Operator would represent an abstract high-level quantum operator, where UnitaryGate would be a low-level primitive unitary for backends that support it (e.g. Aer). qiskit.quantum_info would exclusively deal with Operator, which would be then be lowered by high-level gate decompositions to the target ISA, either by full synthesis (a place we currently use UnitaryGate as a temporary, but Operator could/should replace it), or by the trivial synthesis Operator -> UnitaryGate if the backend ISA includes the variadic unitary instruction (as Aer does).

The Instruction interface (which UnitaryGate satisfies and Operator doesn't) is more constraining than the Operation interface (which Operator does satisfy), and I think it's best to keep both concepts of low- and high-level descriptions, so that backends, and consequently the Target ISA descriptor, etc, need only deal with the low level.

(As a very much side note, can we stop referring to an "endianness" convention? It's at best misleading terminology, and it's not strictly related to the concepts here; the convention for "which qubit is the control?" is a separate question to "in which order do we perform the Kronecker product?" and "in which order do we label qubits?", and none of those are strictly about endianness.)

@ajavadia
Copy link
Member

ajavadia commented Jan 3, 2024

I think all I'm saying for the Operator vs. UnitaryGate thing is that we should make it similar to how Cliffords are done now:

>>>c = QuantumCircuit(2)
>>>c.append(random_clifford(2), [0, 1])
>>>c.append(random_unitary(4), [0, 1])
>>>print(type(c[0]))
>>>print(type(c[1]))
qiskit.quantum_info.operators.symplectic.clifford.Clifford
qiskit.circuit.library.generalized_gates.unitary.UnitaryGate

The first one just uses Clifford, which is far better in my opinion as it gives access to the machinery we have around analyzing Cliffords and doesn't create unnecessary classes. The second one converts Operator -> UnitaryGate under the hood, which is unnecessary IMHO. If we allow CircuitInstructions to point to some quantum_info object, then we immediately allow very general mathematical oeprators to be dropped into circuits, and then teach the transpiler, simulators, etc. to deal with them. The fact that Aer knows about UnitaryGate right now isn't fundamental, it can be adapted to work with CircuitInstruction(operation=Operator(...), ...)

Ok we can call it something other than endianness :) I was trying to say that U.control() should become I \otimes |0><0| + U \otimes |1><1| which is counter-intuitive but makes sense if we consider the original operation as U(t1, t2, t3) and the new one as controlled-U(c, t1, t2, t3), i.e. the 0th qubit is the control, i.e. the least-significant-bit. This is a combination of kronecker product ordering and how we order the qargs of an instruction.

@jakelishman
Copy link
Member

Oh yeah, I totally agree with all that - these to me are examples of a user interacting with the objects in an abstract mathematical way, so it's preferable that they stay as quantum_info objects.

Operator doesn't have a control method yet, so we can give it whatever semantics we like when we add it, right? I totally agree that we should cause Operator.control and UnitaryGate.control to produce the same control-qubit ordering as each other in their matrix forms - it'd be super confusing to get them reversed.

@alexanderivrii
Copy link
Contributor Author

alexanderivrii commented Jan 10, 2024

Supporting Operator.control would also be very easy, since we already have the implementation in Qiskit:
_compute_control_matrix(base_mat, num_ctrl_qubits, ctrl_state=None) incircuit/utils.py (and used in UnitaryGate.control). I will add this to #11534.

Update: though let me first make sure that everyone agrees that the method Operator.control(num_ctrl_qubits, ctrl_state=None) does make sense as a part of Operator class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants