Added to_gate method on certain operators #1299
Conversation
shifubear
requested review from
levbishop,
manoelmarques,
pbark,
stefan-woerner and
woodsp-ibm
as code owners
|
So it seems like the error I'm getting from the latest version of pylint is a known bug: pylint-dev/pylint#3537 Can I just ignore this warning and assume everything is working correctly? |
That is this line below right... use the Python |
|
I've fixed the sections of concern highlighted by @Cryoris and @woodsp-ibm and also added the to_gate operator on the three other gates. Are there any other gates that need this operator that I might have missed? |
shifubear
changed the title
| @@ -218,5 +219,9 @@ def to_matrix_op(self, massive: bool = False) -> OperatorBase: | |||
| def to_instruction(self) -> Instruction: | |||
| return (self.coeff * self.primitive).to_instruction() # type: ignore | |||
|
|
|||
| def to_gate(self, label=None) -> Gate: | |||
| """ Returns a ``Gate`` equivalent to this Operator. """ | |||
| return UnitaryGate(self.primitive, label=label) | |||
There was a problem hiding this comment.
Shouldn't this take care of self.coeff and factor it into self.primitive similar to how this is done in to_instruction above.
Can you can check this aspect on the other to_gate implementations. It maybe for some like circuit_op, like in its to_instruction, that its not feasible to include the coeff on the conversion.
| """Test MatrixOp.to_gate yields a Gate object.""" | ||
| matop = (H ^ 3).to_matrix_op() | ||
| with self.subTest('assert to_gate returns Gate'): | ||
| self.assertIsInstance(matop.to_gate(), Gate) |
There was a problem hiding this comment.
Is there some check we can do on the gate to make sure that the conversion code in to_gate() is working beyond just checking its type. Can we check the value in some way - for instance doing to_matrix on the gate and assuring this is as expected too, or whatever seems reasonable to check that the gate is what we expect beyond just checking type.
|
@Cryoris This I think, if its still wanted needs to be done on the ported opflow now in terra repo but closed off here. Do you have an equivalent issue to #1091 in Terra if this is still wanted there? |
|
This has not been worked on in some time and needs work to complete it. Since the opflow is now in Terra I am moving the issue over there, to be potentially addressed at some point, and closing this PR. @shifubear if you are still interested in doing this and completing the work it will need to be completed over in the opflow in Terra. |
Summary
This pull request resolves issue #1091. I implemented a to_gate method for the following operators:
Details and comments
I added the operator to a circuit and created a gate from that circuit. Is this the correct way to go about it moving forward?
I looked through the other operators, but I couldn't find any others that had the to_instruction method implemented. If there are any more that need this method, I'd be happy to implement this on those.
I added tests for MatrixOp, but there weren't existing tests for the to_instruction method for the other three operators. I was wondering if these need some extra tests, in addition to a test for this added method.