trbromley
commented
1 year ago This bug should be fixed by carrying out the following changes:
- Update
has_unitary_generatorinqml.ops.qubit.attributesto exclude operations whose generator is not a unitary, i.e.,IsingXY,SingleExcitation,DoubleExcitation,OrbitalRotation,FermionicSWAP(second set of operations mentioned above) - Fix the usage in qml.ops.op_math.exp. We should instead iterate over all operations and filter out those which do not have a generator, e.g., by doing something like
for op_name in qml.ops.qubit.__all__, usingop = getattr(qml.ops.qubit, op_name)and postselecting onop.has_generator.
ejthomas
Expected behavior
The name of an object in
qml.ops.qubit.attributescan be understood easily without causing confusion and uses standard terminology.Actual behavior
The attribute
has_unitary_generatoris 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 generator $G$ that satisfies the equation $(-2G) (-2G^\dagger)=4GG^\dagger=\mathbb{I}$, i.e. the generator is unitary up to a prefactor $2$ (or some other complex number with magnitude $2$, that's not noticeable). For this group of operations it is somewhat sensible to call them unitarily generated.IsingXY,SingleExcitation,DoubleExcitation,OrbitalRotation,FermionicSWAPdo not satisfy the above equation. Indeed, all these operations have a generator that has the eigenvalue $0$, a rather non-unitary property.Additional 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
PhaseShiftas non-unitarily generated operation because of the $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.has_unitary_generatoris used in the following two places:qml.transforms.expand_nonunitary_gen, which in turn is only used bymetric_tensorwith the kwargallow_nonunitary=False. However, the criterion by whichmetric_tensorshould decompose in its preprocessing is the question of whetherqml.transforms.metric_tensor._get_gen_ophas a hard-coded support for the respective operation. This means that the flawed registration of non-unitarily generated operations affectsmetric_tensorwithallow_nonunitary=Falsebut the logic anyways is flawed for this decomposition. Removing operations of the second group above fromhas_unitary_generatorwill actually unlock the metric tensor withallow_unitary=Falseqml.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