-
Notifications
You must be signed in to change notification settings - Fork 558
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
[BUG] Operations in has_unitary_generator
do not really have unitary generators
#4055
Comments
This bug should be fixed by carrying out the following changes:
|
Hi, I'd like to work on this issue please 🙂 |
Hi @ejthomas! That's awesome, you can check out our development guide or just post here if you have any questions. Good luck! |
Hi all, I've opened a PR for this fix here (#4183). Interested to hear your thoughts - I hope I've understood correctly what was intended but happy to make any changes needed. Thanks! |
Hi, thanks for assigning me on this issue. I noticed that the issue has been reopened - is there anything else I need to do? It seems that to be counted on UnitaryHack before today's deadline the issue needs to be closed. |
Hi @ejthomas! Congrats on getting this merged and thank you for being a contributor to PennyLane! 🚀 As part of our 0.31 release, we're planning to promote UnitaryHack contributions to PennyLane on social media. This is completely optional, but would you like to be mentioned in our social media posts? Specifically, we'd need either your Twitter profile, your LinkedIn profile, or your name. |
Thanks! That's great, I'm totally happy to be mentioned - my LinkedIn is https://www.linkedin.com/in/edward-thomas-902b17a4 (Edward Thomas) |
Expected behavior
The name of an object in
qml.ops.qubit.attributes
can be understood easily without causing confusion and uses standard terminology.Actual behavior
The attribute
has_unitary_generator
is confusing and does not adhere to the standard notion of "unitary" in physics/math.It contains the following two sets of operations:
RX
,RY
,RZ
,MultiRZ
,PauliRot
,IsingXX
,IsingYY
,IsingZZ
,SingleExcitationMinus
,SingleExcitationPlus
,DoubleExcitationMinus
,DoubleExcitationPlus
,PauliRot
. These operations have a generatorIsingXY
,SingleExcitation
,DoubleExcitation
,OrbitalRotation
,FermionicSWAP
do not satisfy the above equation. Indeed, all these operations have a generator that has the eigenvalueAdditional information
The cause of this mismatch likely is in the discrepancy between the naming of the attribute collection and its use case in the codebase. However, the docstring is very clear about considering$0$ eigenvalue of its generator, which means that the second set of operations mentioned above definitely should not be in there, or both the docstring and the naming need to be changed.
PhaseShift
as non-unitarily generated operation because of thehas_unitary_generator
is used in the following two places:qml.transforms.expand_nonunitary_gen
, which in turn is only used bymetric_tensor
with the kwargallow_nonunitary=False
. However, the criterion by whichmetric_tensor
should decompose in its preprocessing is the question of whetherqml.transforms.metric_tensor._get_gen_op
has a hard-coded support for the respective operation. This means that the flawed registration of non-unitarily generated operations affectsmetric_tensor
withallow_nonunitary=False
but the logic anyways is flawed for this decomposition. Removing operations of the second group above fromhas_unitary_generator
will actually unlock the metric tensor withallow_unitary=False
qml.ops.op_math.exp
, where unitarity does not play a significant role, but we likely should rather be using something along the lines of an attributehas_generator_types
.Source code
No response
Tracebacks
No response
System information
Existing GitHub issues
The text was updated successfully, but these errors were encountered: