Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

More powerful Operator.to_circuit #1089

Closed
Cryoris opened this issue Jul 1, 2020 · 5 comments · Fixed by #1124
Closed

More powerful Operator.to_circuit #1089

Cryoris opened this issue Jul 1, 2020 · 5 comments · Fixed by #1124
Labels
type: feature request New feature or request

Comments

@Cryoris
Copy link
Contributor

Cryoris commented Jul 1, 2020

What is the expected behavior?

Right now, the to_circuit method only works on the operator primitives but not on ListOps. I think we should try to add this method for the ListOps too. If in some cases it is not possible, e.g. because the operator is not unitary, we can raise an error.

Example:

from qiskit.circuit.library import RXGate 
from qiskit.aqua.operators import MatrixOp, X

i = MatrixOp([[1, 0], [0, 1]])
print((i @ X).to_circuit())  # AttributeError: 'ComposedOp' object has no attribute 'to_circuit'
print((i ^ X).to_circuit())  # AttributeError: 'TensoredOp' object has no attribute 'to_circuit'

Though i.to_circuit and X.to_circuit both work.

Note that some combinations, such as CircuitOp @ PauliOp, work because the composition does not return a ComposedOp but the type of the left-hand-side operator.
@Cryoris Cryoris added the type: feature request New feature or request label Jul 1, 2020
@molar-volume
Copy link
Contributor

Hi @Cryoris
I implemented a simple solution that works on ListOps

out

and raises error for non-unitary operators

err

I will create PR as soon as I extend the tests.

@molar-volume molar-volume mentioned this issue Jul 20, 2020
Merged
@jlapeyre
Copy link
Contributor

Thanks for looking into this.
Please do not paste images of code. Rather use quoted code blocks as in the first comment in this issue.

@jlapeyre
Copy link
Contributor

@Cryoris Referring to the opening comment in this issue: Are you suggesting supporting to_circuit for derived classes only (eg SummedOp) or also for instances of ListOp ? The latter is meant to be container supporting vectorized operations.

@Cryoris
Copy link
Contributor Author

Cryoris commented Jul 21, 2020

The idea was to add it only for derived classes, meaning: TensoredOp, ComposedOp and SummedOp.

If we want we could also add it to the ListOp itself but then it should return (nested) lists of circuits. But maybe let's start with only doing it for the derived classes.

@molar-volume
Copy link
Contributor

@Cryoris I moved the methods to the child classes, please see the PR.

@woodsp-ibm woodsp-ibm mentioned this issue Jul 23, 2020
Closed
woodsp-ibm added a commit that referenced this issue Aug 21, 2020
@molar-volume @Cryoris
@manoelmarques @woodsp-ibm
More powerful Operator.to_circuit #1089 (#1124)
1d8df63 
* 1) for ListOp, new method ‘to_circuit’ implemented, using existing to_circuit_op
2) for SummedOp, ‘to_circuit’ overriden to avoid infinite recursion

* tests for ListOp.to_circuit

* 1) to_circuit method moved from ListOp to child classes
2) removed unnecessary abstract to_circuit from OperatorBase
* Update test/aqua/operators/test_op_construction.py

* verify that ListOp.to_circuit() outputs correct quantum circuit

* Check if to_circuit_op returns expected operator to avoid the #type: ignore.
Else, exception is raised

* test for to_circuit method for SummedOp([TensoredOp, TensoredOp])

* tests exposing the missing features in to_circuit, to_instruction and to_matrix of PrimitiveOps with Parameter

* 1) SummedOp.to_circuit raises exception, if conversion to_matrix_op does not provide MatrixOp (which happens if respective summands are parametrized)
2) added unit test to cover this case

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Manoel Marques <manoel@us.ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
Cryoris added a commit to Cryoris/qiskit-aqua that referenced this issue Sep 3, 2020
@molar-volume @Cryoris
@manoelmarques @woodsp-ibm
More powerful Operator.to_circuit qiskit-community#1089 (qiskit-commu…
01eadf8 
…nity#1124)

* 1) for ListOp, new method ‘to_circuit’ implemented, using existing to_circuit_op
2) for SummedOp, ‘to_circuit’ overriden to avoid infinite recursion

* tests for ListOp.to_circuit

* 1) to_circuit method moved from ListOp to child classes
2) removed unnecessary abstract to_circuit from OperatorBase
* Update test/aqua/operators/test_op_construction.py

* verify that ListOp.to_circuit() outputs correct quantum circuit

* Check if to_circuit_op returns expected operator to avoid the #type: ignore.
Else, exception is raised

* test for to_circuit method for SummedOp([TensoredOp, TensoredOp])

* tests exposing the missing features in to_circuit, to_instruction and to_matrix of PrimitiveOps with Parameter

* 1) SummedOp.to_circuit raises exception, if conversion to_matrix_op does not provide MatrixOp (which happens if respective summands are parametrized)
2) added unit test to cover this case

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Manoel Marques <manoel@us.ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
pbark pushed a commit to pbark/qiskit-aqua that referenced this issue Sep 16, 2020
@molar-volume @Cryoris
@manoelmarques @woodsp-ibm @pbark
More powerful Operator.to_circuit qiskit-community#1089 (qiskit-commu…
27976e1 
…nity#1124)

* 1) for ListOp, new method ‘to_circuit’ implemented, using existing to_circuit_op
2) for SummedOp, ‘to_circuit’ overriden to avoid infinite recursion

* tests for ListOp.to_circuit

* 1) to_circuit method moved from ListOp to child classes
2) removed unnecessary abstract to_circuit from OperatorBase
* Update test/aqua/operators/test_op_construction.py

* verify that ListOp.to_circuit() outputs correct quantum circuit

* Check if to_circuit_op returns expected operator to avoid the #type: ignore.
Else, exception is raised

* test for to_circuit method for SummedOp([TensoredOp, TensoredOp])

* tests exposing the missing features in to_circuit, to_instruction and to_matrix of PrimitiveOps with Parameter

* 1) SummedOp.to_circuit raises exception, if conversion to_matrix_op does not provide MatrixOp (which happens if respective summands are parametrized)
2) added unit test to cover this case

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Manoel Marques <manoel@us.ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
mtreinish pushed a commit to mtreinish/qiskit-core that referenced this issue Nov 20, 2020
@molar-volume @Cryoris
@manoelmarques @woodsp-ibm
More powerful Operator.to_circuit qiskit-community/qiskit-aqua#1089 (q…
778ad31 
…iskit-community/qiskit-aqua#1124)

* 1) for ListOp, new method ‘to_circuit’ implemented, using existing to_circuit_op
2) for SummedOp, ‘to_circuit’ overriden to avoid infinite recursion

* tests for ListOp.to_circuit

* 1) to_circuit method moved from ListOp to child classes
2) removed unnecessary abstract to_circuit from OperatorBase
* Update test/aqua/operators/test_op_construction.py

* verify that ListOp.to_circuit() outputs correct quantum circuit

* Check if to_circuit_op returns expected operator to avoid the #type: ignore.
Else, exception is raised

* test for to_circuit method for SummedOp([TensoredOp, TensoredOp])

* tests exposing the missing features in to_circuit, to_instruction and to_matrix of PrimitiveOps with Parameter

* 1) SummedOp.to_circuit raises exception, if conversion to_matrix_op does not provide MatrixOp (which happens if respective summands are parametrized)
2) added unit test to cover this case

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Manoel Marques <manoel@us.ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
manoelmarques added a commit to manoelmarques/qiskit-terra that referenced this issue Dec 2, 2020
@molar-volume @Cryoris
@manoelmarques @woodsp-ibm
More powerful Operator.to_circuit qiskit-community/qiskit-aqua#1089 (q…
7cfa3ef 
…iskit-community/qiskit-aqua#1124)

* 1) for ListOp, new method ‘to_circuit’ implemented, using existing to_circuit_op
2) for SummedOp, ‘to_circuit’ overriden to avoid infinite recursion

* tests for ListOp.to_circuit

* 1) to_circuit method moved from ListOp to child classes
2) removed unnecessary abstract to_circuit from OperatorBase
* Update test/aqua/operators/test_op_construction.py

* verify that ListOp.to_circuit() outputs correct quantum circuit

* Check if to_circuit_op returns expected operator to avoid the #type: ignore.
Else, exception is raised

* test for to_circuit method for SummedOp([TensoredOp, TensoredOp])

* tests exposing the missing features in to_circuit, to_instruction and to_matrix of PrimitiveOps with Parameter

* 1) SummedOp.to_circuit raises exception, if conversion to_matrix_op does not provide MatrixOp (which happens if respective summands are parametrized)
2) added unit test to cover this case

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Manoel Marques <manoel@us.ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
manoelmarques added a commit to manoelmarques/qiskit-terra that referenced this issue Dec 7, 2020
@molar-volume @Cryoris
@manoelmarques @woodsp-ibm
More powerful Operator.to_circuit qiskit-community/qiskit-aqua#1089 (q…
3d194ab 
…iskit-community/qiskit-aqua#1124)

* 1) for ListOp, new method ‘to_circuit’ implemented, using existing to_circuit_op
2) for SummedOp, ‘to_circuit’ overriden to avoid infinite recursion

* tests for ListOp.to_circuit

* 1) to_circuit method moved from ListOp to child classes
2) removed unnecessary abstract to_circuit from OperatorBase
* Update test/aqua/operators/test_op_construction.py

* verify that ListOp.to_circuit() outputs correct quantum circuit

* Check if to_circuit_op returns expected operator to avoid the #type: ignore.
Else, exception is raised

* test for to_circuit method for SummedOp([TensoredOp, TensoredOp])

* tests exposing the missing features in to_circuit, to_instruction and to_matrix of PrimitiveOps with Parameter

* 1) SummedOp.to_circuit raises exception, if conversion to_matrix_op does not provide MatrixOp (which happens if respective summands are parametrized)
2) added unit test to cover this case

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Manoel Marques <manoel@us.ibm.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
type: feature request New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

More powerful Operator.to_circuit #1089 molar-volume/qiskit-aqua
3 participants
@jlapeyre @Cryoris @molar-volume