Proposal - feat: The Quil constant pi can now be represented directly with the new Pi class

[MarquessV]

Description

This is an attempt to mitigate the issues in #1664 where parsing a program containing the string pi would result in a program containing the Quil constant pi but programatically building a program from PyQuil classes containing a float like np.pi would result in expressions containing the literal float 3.14.... This results in what are effectively equivalent programs not being evaluated as such.

My proposal here is to add a new Pi class to the expression module that can be used to represent the Quil constant pi. As a descendant of the Expression class, it can be used as part of most kinds of Python expressions easily.

One limitation is that Python expressions between Expression objects result in an Expression. This can be problematic in situations where an atomic type is needed. In order to support that, I've added support for casting an Expression to an int, float, or complex value is supported. This leans on quil to simplify the expression, and will fail if the expression contains any unbound parameters.

Adding casting makes it easier to use Expressions in numpy array's, but note that once used inside of an array, the expression will be cast to an atomic value and you will lose information about the expression (e.g. if it is a "pi" constant).

Checklist

• The PR targets the master branch
• The above description motivates these changes.
• The change is atomic and can be described by a single commit (your PR will be squashed on merge).
• All changes to code are covered via unit tests.
• Parameters and return values have type hints with PEP 484 syntax.
• Functions and classes have useful Sphinx-style docstrings.
• (New Feature) The docs have been updated accordingly.

[MarquessV]

@bramathon curious what you think about this proposal. Are the limitations workable, or would you need a solution for them before this could really be adopted?

[kalzoo]

This looks uncontroversial to me! Interested to hear the user perspective as well.

[kalzoo]

Is this necessary? This feels like we might regret having to maintain it

[kalzoo]

Worth noting, quil-rs should do this itself, but I can see wanting the extra assurance here

[kalzoo]

Not changes requested, but for your awareness in case it comes up in the near future - expression simplification can be expensive.

[bramathon]

In pyquil v3, the constant pi was a float. Specifically, the program constructed like so,

import numpy as np
from pyquil.quil import Program
from pyquil.gates import RX

program = Program()
program += RX(np.pi/2, 0)
program.instructions[0].params[0]

would yield a parameter value of 1.5707963267948966, however it would be printed as:

RX(pi/2) 0

With pyquil 4, the identical construction led to a program string of:

RX(1.5707963267948966) 0

while parsing the Quil

RX(pi/2) 0

produces a gate with a scalar float of 1.5707963267948966, but the program retains the string representation using the symbol pi.

With this proposal, I understand that the gate parameter would become an expression Pi rather than a scalar value 1.5707963267948966 and program constructed above would once again print as

RX(pi/2) 0

I believe this change would cause more problems than it would solve. Pyquil expressions are not typically convenient objects to work with, particular for vectorized code. It would be preferable to me to simply maintain the pyquil 3 behaviour, where scalar values near fractions of pi are represented as so, but if that is not possible then they should always be floats.

[kalzoo]

In pyquil v3, the constant pi was a float. Specifically, the program constructed like so,

import numpy as np
from pyquil.quil import Program
from pyquil.gates import RX

program = Program()
program += RX(np.pi/2, 0)
program.instructions[0].params[0]

would yield a parameter value of 1.5707963267948966, however it would be printed as:

RX(pi/2) 0

With pyquil 4, the identical construction led to a program string of:

RX(1.5707963267948966) 0

while parsing the Quil

RX(pi/2) 0

produces a gate with a scalar float of 1.5707963267948966, but the program retains the string representation using the symbol pi.

With this proposal, I understand that the gate parameter would become an expression Pi rather than a scalar value 1.5707963267948966 and program constructed above would once again print as

RX(pi/2) 0

I believe this change would cause more problems than it would solve. Pyquil expressions are not typically convenient objects to work with, particular for vectorized code. It would be preferable to me to simply maintain the pyquil 3 behaviour, where scalar values near fractions of pi are represented as so, but if that is not possible then they should always be floats.

It seems like between the issue and this comment, the concern moves between two different things:

1. use of an expression value as a float
2. display of the expression value

It seems to me like this MR implementation meets your targets for both, right?

• If you want pi to be special - for readable output and equivalence - it can be
• If you want pi to be a float - for computation, vectorization, etc - it can be that too, with float

The only catch is that as a user you have to ask for the behavior you want. As the vendors of the library, that seems like the right choice to keep different users happy with different preferences. Right?

It would be preferable to me to simply maintain the pyquil 3 behaviour, where scalar values near fractions of pi are represented as so

This, to me, was always hacky / "magical" in that we were trying to infer the user's intent, but only in some cases, and only for the convenience of readability. That is, maybe we do pi and n*pi and pi/2, but what about 5pi/6 or pi/5? That exposes the arbitrariness of it, not to mention the extra computation to print an instruction or program.

[bramathon]

Changing pi to be an expression will cause a tremendous number of bugs, because to date we have assumed that expressions are parametric while fixed values are scalars, and a lot of code relies on this being true.

From my perspective, printing decimal values as fractions of pi is a nice-to-have. It makes the quil much more readable and since RX(pi/2) is pretty much the most common gate, it is something that comes up a lot.

What is more important is that:

1. Identical quil programs serialize identically.
2. Scalar gate parameter values are python float, complex, or int.

That is, maybe we do pi and n*pi and pi/2, but what about 5pi/6 or pi/5?

Sure, why not?

import numpy as np
from fractions import Fraction

def to_pi_frac(x: float) -> str:
    """Convert a float to a fraction of pi with a denominator <64."""
    frac = Fraction(x / np.pi).limit_denominator(64)
    if not np.isclose(float(frac)*np.pi, x, atol=1e-6):
        return x
    return ("𝜋" if frac.numerator == 1 else f"{frac.numerator}𝜋") + ("" if frac.denominator == 1 else f"/{frac.denominator}")

[mhodson-rigetti]

My first reaction was that a symbolic pi was a bad idea, but on more careful inspection of @kalzoo 's arguments, I find them sound. I do wonder whether there would need to be support for this in other places, like quil-rs expression simplification, but that might already be A-OK.

@bramathon there's two flaws in your argument:

(1) The statement that the change "will cause a tremendous number of bugs, because to date we have assumed that expressions are parametric while fixed values are scalars, and a lot of code relies on this being true" is by definition not a bug in pyquil, its a limitation of the down-stream code that would need to be addressed.

(2) Your code example that determines a pi conversion is limited to denominators up to 64, and that kind of makes @kalzoo 's point for him. It cannot work in the general case.

I agree with the round-trip requirements but I assume they would hold; if we wrote using pyquil.pi we'd see text "pi" in the Quil and it would round trip. If we used np.pi we'd see the float 3.142... to 16 decimal places in the output, and it would round trip.

[bramathon]

(1) The statement that the change "will cause a tremendous number of bugs, because to date we have assumed that expressions are parametric while fixed values are scalars, and a lot of code relies on this being true" is by definition not a bug in pyquil, its a limitation of the down-stream code that would need to be addressed.

Yes, sorry the introduction of the pi symbol is clearly not a bug in pyquil, but clearly an intentional change in behaviour. The "bugs" will appear in downstream packages which relied (correctly or not) on the assumption that scalar values were always numerical types.

(2) Your code example that determines a pi conversion is limited to denominators up to 64, and that kind of makes @kalzoo 's point for him. It cannot work in the general case.

I understand the goal of the pi constant is to improve readability. I set the maximum denominator to 64 because I believe that fractions beyond that do not improve readability. This is, of course, subjective, but the point is merely that we only need to agree on some rule and follow it. (yes this would result in something like the string pi/128 being converted to a float, but this is a very marginal case).

The crux of the issue is that as a user, my preferences are in this order:

1. Identical programs serialize identically
2. Scalar parameter values are numeric
3. Readability

One thing I do not rely on is the round-trip requirement, although it could be important elsewhere.