pennylane icon indicating copy to clipboard operation
pennylane copied to clipboard

Identity accepts multiple wires.

Open AlbertMitjans opened this issue 2 years ago • 16 comments

The qml.Identity class can now be instantiated with multiple wires:

>>> id_op = qml.Identity([0, 1])
>>> id_op.matrix()
array([[1., 0., 0., 0.],
       [0., 1., 0., 0.],
       [0., 0., 1., 0.],
       [0., 0., 0., 1.]])
>>> id_op.sparse_matrix()
<4x4 sparse matrix of type '<class 'numpy.float64'>'
	with 4 stored elements in Compressed Sparse Row format>
>>> id_op.eigvals()
array([1., 1., 1., 1.])

AlbertMitjans avatar Sep 12 '22 15:09 AlbertMitjans

Hello. You may have forgotten to update the changelog! Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

github-actions[bot] avatar Sep 12 '22 15:09 github-actions[bot]

Codecov Report

Merging #3049 (ea6d4b4) into master (318d004) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3049   +/-   ##
=======================================
  Coverage   99.68%   99.68%           
=======================================
  Files         274      274           
  Lines       23864    23864           
=======================================
  Hits        23790    23790           
  Misses         74       74           
Impacted Files Coverage Δ
pennylane/operation.py 97.00% <100.00%> (-0.01%) :arrow_down:
pennylane/ops/identity.py 100.00% <100.00%> (ø)
pennylane/ops/op_math/pow.py 100.00% <100.00%> (ø)
pennylane/ops/op_math/prod.py 100.00% <100.00%> (ø)
pennylane/ops/op_math/sum.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Sep 13 '22 10:09 codecov[bot]

[sc-25912]

AlbertMitjans avatar Sep 13 '22 14:09 AlbertMitjans

Hi @AlbertMitjans, a fly-by comment: we used to allow this but for certain reasons IIRC including the CV cases we've disallowed it.

Have we converged to a point where we definitely want to re-allow this syntax (e.g., with the op arithmetic push)? qml.Identity taking multiple wires duplicates behaviour we could achieve by using qml.operation.Tensor (and I guess the new product class, though may be wrong here) and multiple qml.Identity objects.

antalszava avatar Sep 19 '22 13:09 antalszava

Note also the discussion here, which is somewhat related: https://github.com/PennyLaneAI/pennylane/issues/2563

Although, this is maybe slightly different; rather than converting qml.Identity(wires=[1, 2, 3]) to qml.Identity(0) @ qml.Identity(1) @ qml.Identity(2) like the issue suggests, we are instead allowing qml.Identity to support multiple wires natively. Which makes more sense.

josh146 avatar Sep 19 '22 13:09 josh146

is the goal here primarily to make prod work better with identity? if so, is there a reason we need to ensure there is more than one operand passed in to the Prod constructor? Just having a Prod instance with operands [qml.Identity(wires=0),] seems not too bad to me

timmysilv avatar Sep 19 '22 14:09 timmysilv

If a device or a compilation step sees an Identity, it knows that it can just skip over the operation and neglect it. See default_qubit.py line 276 as an example.

If we instead have a product on identities, that "skipping" is much harder to perform.

For example, what if someone performed:

>>> qml.simplify(qml.CNOT((0,1)) ** 2)
Identity(wires=[0]) @ Identity(wires=[1])

We should be able to identify that as an Identity operation easily.

We could also just add a MultiIdentity gate for these cases.

albi3ro avatar Sep 19 '22 17:09 albi3ro

Hi @AlbertMitjans, a fly-by comment: we used to allow this but for certain reasons IIRC including the CV cases we've disallowed it.

Have we converged to a point where we definitely want to re-allow this syntax (e.g., with the op arithmetic push)? qml.Identity taking multiple wires duplicates behaviour we could achieve by using qml.operation.Tensor (and I guess the new product class, though may be wrong here) and multiple qml.Identity objects.

Could you elaborate more on why it was disallowed?

And yes, there will be duplicated behaviour, meaning that qml.Identity([0, 1, 2]) would be the same as qml.prod(qml.Identity(0), qml.Identity(1), qml.Identity(2)), but I don't see this as a problem.

AlbertMitjans avatar Sep 20 '22 08:09 AlbertMitjans

If a device or a compilation step sees an Identity, it knows that it can just skip over the operation and neglect it. See default_qubit.py line 276 as an example.

If we instead have a product on identities, that "skipping" is much harder to perform.

For example, what if someone performed:

>>> qml.simplify(qml.CNOT((0,1)) ** 2)
Identity(wires=[0]) @ Identity(wires=[1])

We should be able to identify that as an Identity operation easily.

We could also just add a MultiIdentity gate for these cases.

What approach should we follow then? Do we keep the current modifications, or should I create a new class?

AlbertMitjans avatar Sep 20 '22 08:09 AlbertMitjans

Could you elaborate more on why it was disallowed?

git blame showed that we changed it when qml.Identity became an operation: https://github.com/PennyLaneAI/pennylane/pull/1829/

Maybe @Jaybsoni has more context on the change made at the time. Also, for that reason, qml.Identity([0, 1, 2]) has to be considered as an operation too (unless it's being skipped).

And yes, there will be duplicated behaviour, meaning that qml.Identity([0, 1, 2]) would be the same as qml.prod(qml.Identity(0), qml.Identity(1), qml.Identity(2)), but I don't see this as a problem.

I'm not a big fan of that personally because allowing the representation of the same observables in two different ways in such a way can lead to bugs more easily. For example, if a developer has to check that the observable is an identity or a tensor product of identities, they have to be aware and remember that there are two ways of representing such observables.

Could it be an option to make qml.Identity([0, 1, 2]) resolve to qml.prod(qml.Identity(0), qml.Identity(1), qml.Identity(2)) (or the other way round)`?

antalszava avatar Sep 20 '22 16:09 antalszava

Could it be an option to make qml.Identity([0, 1, 2]) resolve to qml.prod(qml.Identity(0), qml.Identity(1), qml.Identity(2)) (or the other way round)`?

One thing I was planning to do was resolving qml.prod(qml.Identity(0), qml.Identity(1), qml.Identity(2)) to qml.Identity([0, 1, 2]) in the simplify pipeline.

Duplicated behaviour is very common in operator arithmetic (e.g. qml.sum(qml.PauliX(0), qml.PauliX(0)) is the same as qml.s_prod(2, qml.PauliX(0))...) . I think it's fine as long as after simplification we have a unique representation for all operators.

AlbertMitjans avatar Sep 21 '22 07:09 AlbertMitjans

We can always make this change and then revert it later if it ends up being problematic and buggy.

We can add a decomposition, so that:

>>> Identity((0,1)).decomposition()
[Identity(0), Identity(1)]

albi3ro avatar Sep 21 '22 13:09 albi3ro

Duplicated behaviour is very common in operator arithmetic (e.g. qml.sum(qml.PauliX(0), qml.PauliX(0)) is the same as qml.s_prod(2, qml.PauliX(0))...) . I think it's fine as long as after simplification we have a unique representation for all operators.

:+1: I think that sound reasonable. We used to not have such simplification in the past.

Would that imply that it's encouraged to call qml.simplify on arbitrary operators before performing such checks? Does that have any performance implications?

antalszava avatar Sep 21 '22 15:09 antalszava

We can always make this change and then revert it later if it ends up being problematic and buggy.

We can add a decomposition, so that:

>>> Identity((0,1)).decomposition()
[Identity(0), Identity(1)]

Decomposition was already used, and it returned an empty list. I believe this is what we want right? When expanding the qml.Identity operator, we basically skip it.

AlbertMitjans avatar Sep 22 '22 08:09 AlbertMitjans

Duplicated behaviour is very common in operator arithmetic (e.g. qml.sum(qml.PauliX(0), qml.PauliX(0)) is the same as qml.s_prod(2, qml.PauliX(0))...) . I think it's fine as long as after simplification we have a unique representation for all operators.

👍 I think that sound reasonable. We used to not have such simplification in the past.

Would that imply that it's encouraged to call qml.simplify on arbitrary operators before performing such checks? Does that have any performance implications?

I would not recommend using qml.simplify inside the PL source code, because it is quite expensive. qml.simplify should be a tool for the users. If users want to use a product of identities inside an operator, then they should call qml.simplify before executing it in a circuit, to make sure that all identities are skipped. I can add this in the docs if needed.

For us developers, we should avoid using a product of identities, and use qml.Identity instead.

AlbertMitjans avatar Sep 22 '22 08:09 AlbertMitjans

I just realised that the identity simplification already works! I love when this happens hahaha:

>>> id_op = qml.prod(qml.Identity(0), qml.Identity(1), qml.Identity(2))
>>> qml.simplify(id_op)
Identity(wires=[0, 1, 2])

AlbertMitjans avatar Sep 22 '22 09:09 AlbertMitjans