Skip to content
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

Ensure scalar parameter names are preserved during conversion #6755

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

natestemen
Copy link

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.

@natestemen natestemen requested review from vtomole and a team as code owners October 8, 2024 22:11
Copy link

google-cla bot commented Oct 8, 2024

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.

@natestemen
Copy link
Author

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.

@pavoljuhas
Copy link
Collaborator

@natestemen - it seems the CLA check is failing (the second comment here).
Please fix, otherwise we are not able to consider the PR.

@natestemen
Copy link
Author

Ah sorry about that! Didn't realize it was blocking a review. Resolved now.

@CirqBot CirqBot added the size: S 10< lines changed <50 label Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: S 10< lines changed <50
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants