Cirq
Cirq copied to clipboard
Ensure scalar parameter names are preserved during conversion
Description: When converting Quil programs with single-value (scalar) parameters, ensure that the parameter name remains consistent by returning the symbolic name.
Motivation: When upgrading from pyquil v3 to v4, the output of a simple program changes subtly. If the starting pyquil program is
program = Program()
theta = program.declare("theta", memory_type="REAL")
program += RZ(theta, 0)
Then the diff between v3 program.out() and v4 program.out() is as follows.
-RZ(theta) 0
+RZ(theta[0]) 0
So far, this has nothing to do with cirq, but when calling cirq_rigetti.circuit_from_quil(program.out()) the instantiated cirq.Circuit now has a parameter name theta_0 which was never specified by the user.
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
I was able to reproduce this locally, but the CI failure made me look at it again. It seems that declared_size is not always set (i.e. it depends on how you instantiate the pyquil.Program: either via a quil string, or sequentially building it via pyquil operations). This makes it an unreliable signal for determining whether the variable is actually a scalar.
I'm a bit stumped as to what the best approach would be to resolve the underlying issue. Any advice would be appreciated.
@natestemen - it seems the CLA check is failing (the second comment here). Please fix, otherwise we are not able to consider the PR.
Ah sorry about that! Didn't realize it was blocking a review. Resolved now.
@natestemen - after testing this a bit, I would argue the current cirq parameters naming is more close to the pyquil's program.out() behavior in your initial https://github.com/quantumlib/Cirq/pull/6755#issue-2574314732.
The declaration of the theta scalar creates an array of size 1 so it's value is later referenced as theta[0].
Unless this is a bug in pyquil I feel it is better to leave the code as is. (cc @jselig-rigetti - can you please comment?)
BTW, the test quil code added in this PR renders the same for pyquil 3 and 4 with parameter expression theta[0]. Converting such program to a circuit with parameter theta_0 sounds sensible.
from pyquil import Program
print(Program("DECLARE theta REAL[1]\nRZ(theta) 0"))
output:
DECLARE theta REAL[1]
RZ(theta[0]) 0
The main point of confusion is that parameter names are no longer consistent across SDKs. E.g. I define a a pyquil program with a parameter, I have to do some amount of logic to figure out what the parameter name is in the cirq circuit. That's a surprising behavior IMO.
That said, I understand where you're coming from, and the current implementation keeps things simple. I appreciate you taking a look!