Repository: rigetti/pyquil (python)

Issue:
Parameters to gates that are nearly pi, pi/2, pi/4 etc should be printed using pi
Instead of this:
`RZ(-3.141592653589793) 0`
let's print this:
`RZ(-pi) 0` which is valid quil.

This will aid in program understanding.


Discussion:
Hint to implementers: look at the format_parameter function in quilbase
Hello,

I implemented this last night using np.float32 and fractions.Fraction.  I wanted to verify the expected behavior before making a pull request.  Here are some printed results, which after obvious adjustments to the test cases pass tests for python2 and python3.  
```
input
=========

for i in range(12):
    p = Program()
    p += RZ(i*pi/91.,0)
    print(p)

output
=========
RZ(0) 0
RZ(pi/91) 0
RZ(2*pi/91) 0
RZ(3*pi/91) 0
RZ(4*pi/91) 0
RZ(5*pi/91) 0
RZ(6*pi/91) 0
RZ(pi/13) 0
RZ(8*pi/91) 0
RZ(9*pi/91) 0
RZ(10*pi/91) 0
RZ(11*pi/91) 0
```
The main idea is to use fractions.Fraction to see if np.float32(element/np.pi) == np.float32(p/q) for some fraction p/q with q <= 140 (arbitrary cutoff).  One important edge case to think about is 
```
52*pi/91.  
```
In python2 and python3, 
```
52*pi/91. != 4*pi/7.  
```
This is the main need for considering these subtleties.  
Was this the sort of solution you had in mind?






You could use np.isclose instead of comparing floats directly to deal with your second issue.

This looks good, the only thing I'd change is just reducing q to be much smaller, maybe 8. We only want to pretty print in the simplest of common cases like 3pi/4, pi/8, etc. Anything beyond that doesn't come up very often.

Looking forward to checking out your PR!

Patch:
diff --git a/pyquil/quilbase.py b/pyquil/quilbase.py
--- a/pyquil/quilbase.py
+++ b/pyquil/quilbase.py
@@ -20,6 +20,7 @@
 import numpy as np
 from .slot import Slot
 from six import integer_types, string_types
+from fractions import Fraction
 
 
 class QuilAtom(object):
@@ -513,8 +514,10 @@ def format_parameter(element):
 
     :param element: {int, float, long, complex, Slot} Formats a parameter for Quil output.
     """
-    if isinstance(element, integer_types) or isinstance(element, float):
+    if isinstance(element, integer_types):
         return repr(element)
+    elif isinstance(element, float):
+        return check_for_pi(element)
     elif isinstance(element, complex):
         r = element.real
         i = element.imag
@@ -527,6 +530,33 @@ def format_parameter(element):
     assert False, "Invalid parameter: %r" % element
 
 
+def check_for_pi(element):
+    """
+    Check to see if there exists a rational number r = p/q
+    in reduced form for which the difference between element/np.pi
+    and r is small and q <= 8.
+
+    :param element: float
+    :return element: pretty print string if true, else standard representation.
+    """
+    frac = Fraction(element / np.pi).limit_denominator(8)
+    num, den = frac.numerator, frac.denominator
+    sign = "-" if num < 0 else ""
+    if np.isclose(num / float(den), element / np.pi):
+        if num == 0:
+            return "0"
+        elif abs(num) == 1 and den == 1:
+            return sign + "pi"
+        elif abs(num) == 1:
+            return sign + "pi/" + repr(den)
+        elif den == 1:
+            return repr(num) + "*pi"
+        else:
+            return repr(num) + "*pi/" + repr(den)
+    else:
+        return repr(element)
+
+
 def unpack_qubit(qubit):
     """
     Get a qubit from an object.


Test patch:
diff --git a/pyquil/tests/test_quil.py b/pyquil/tests/test_quil.py
--- a/pyquil/tests/test_quil.py
+++ b/pyquil/tests/test_quil.py
@@ -40,7 +40,7 @@ def test_gate():
 def test_defgate():
     dg = DefGate("TEST", np.array([[1., 0.],
                                    [0., 1.]]))
-    assert dg.out() == "DEFGATE TEST:\n    1.0, 0.0\n    0.0, 1.0\n"
+    assert dg.out() == "DEFGATE TEST:\n    1.0, 0\n    0, 1.0\n"
     test = dg.get_constructor()
     tg = test(Qubit(1), Qubit(2))
     assert tg.out() == "TEST 1 2"
@@ -64,7 +64,7 @@ def test_defgate_non_unitary_should_throw_error():
 
 def test_defgate_param():
     dgp = DefGate("TEST", [[1., 0.], [0., 1.]])
-    assert dgp.out() == "DEFGATE TEST:\n    1.0, 0.0\n    0.0, 1.0\n"
+    assert dgp.out() == "DEFGATE TEST:\n    1.0, 0\n    0, 1.0\n"
     test = dgp.get_constructor()
     tg = test(Qubit(1))
     assert tg.out() == "TEST 1"
@@ -236,21 +236,21 @@ def test_dagger():
                        RZ(pi, 0), CPHASE(pi, 0, 1),
                        CPHASE00(pi, 0, 1), CPHASE01(pi, 0, 1),
                        CPHASE10(pi, 0, 1), PSWAP(pi, 0, 1))
-    assert p.dagger().out() == 'PSWAP(-3.141592653589793) 0 1\n' \
-                               'CPHASE10(-3.141592653589793) 0 1\n' \
-                               'CPHASE01(-3.141592653589793) 0 1\n' \
-                               'CPHASE00(-3.141592653589793) 0 1\n' \
-                               'CPHASE(-3.141592653589793) 0 1\n' \
-                               'RZ(-3.141592653589793) 0\n' \
-                               'RY(-3.141592653589793) 0\n' \
-                               'RX(-3.141592653589793) 0\n' \
-                               'PHASE(-3.141592653589793) 0\n'
+    assert p.dagger().out() == 'PSWAP(-pi) 0 1\n' \
+                               'CPHASE10(-pi) 0 1\n' \
+                               'CPHASE01(-pi) 0 1\n' \
+                               'CPHASE00(-pi) 0 1\n' \
+                               'CPHASE(-pi) 0 1\n' \
+                               'RZ(-pi) 0\n' \
+                               'RY(-pi) 0\n' \
+                               'RX(-pi) 0\n' \
+                               'PHASE(-pi) 0\n'
 
     # these gates are special cases
     p = Program().inst(S(0), T(0), ISWAP(0, 1))
-    assert p.dagger().out() == 'PSWAP(1.5707963267948966) 0 1\n' \
-                               'RZ(0.7853981633974483) 0\n' \
-                               'PHASE(-1.5707963267948966) 0\n'
+    assert p.dagger().out() == 'PSWAP(pi/2) 0 1\n' \
+                               'RZ(pi/4) 0\n' \
+                               'PHASE(-pi/2) 0\n'
 
     # must invert defined gates
     G = np.array([[0, 1], [0 + 1j, 0]])
@@ -294,14 +294,14 @@ def test_phases():
     p = Program(PHASE(np.pi)(1), CPHASE00(np.pi)(0, 1), CPHASE01(np.pi)(0, 1),
                 CPHASE10(np.pi)(0, 1),
                 CPHASE(np.pi)(0, 1))
-    assert p.out() == 'PHASE(3.141592653589793) 1\nCPHASE00(3.141592653589793) 0 1\n' \
-                      'CPHASE01(3.141592653589793) 0 1\nCPHASE10(3.141592653589793) 0 1\n' \
-                      'CPHASE(3.141592653589793) 0 1\n'
+    assert p.out() == 'PHASE(pi) 1\nCPHASE00(pi) 0 1\n' \
+                      'CPHASE01(pi) 0 1\nCPHASE10(pi) 0 1\n' \
+                      'CPHASE(pi) 0 1\n'
 
 
 def test_swaps():
     p = Program(SWAP(0, 1), CSWAP(0, 1, 2), ISWAP(0, 1), PSWAP(np.pi)(0, 1))
-    assert p.out() == 'SWAP 0 1\nCSWAP 0 1 2\nISWAP 0 1\nPSWAP(3.141592653589793) 0 1\n'
+    assert p.out() == 'SWAP 0 1\nCSWAP 0 1 2\nISWAP 0 1\nPSWAP(pi) 0 1\n'
 
 
 def test_def_gate():
@@ -350,8 +350,8 @@ def qft3(q0, q1, q2):
 
     prog = state_prep + qft3(0, 1, 2)
     output = prog.out()
-    assert output == 'X 0\nH 2\nCPHASE(1.5707963267948966) 1 2\nH 1\nCPHASE(0.7853981633974483) 0 ' \
-                     '2\nCPHASE(1.5707963267948966) 0 1\nH 0\nSWAP 0 2\n'
+    assert output == 'X 0\nH 2\nCPHASE(pi/2) 1 2\nH 1\nCPHASE(pi/4) 0 ' \
+                     '2\nCPHASE(pi/2) 0 1\nH 0\nSWAP 0 2\n'
 
 
 def test_control_flows():
@@ -574,3 +574,14 @@ def test_inline_alloc():
     p = Program()
     p += H(p.alloc())
     assert p.out() == "H 0\n"
+
+
+# https://github.com/rigetticomputing/pyquil/issues/184
+def test_pretty_print_pi():
+    p = Program()
+    p += [RZ(0., 0), RZ(pi, 1), RZ(-pi, 2)]
+    p += [RZ(2 * pi / 3., 3), RZ(pi / 9., 4), RZ(pi / 8., 5)]
+    p += CPHASE00(-90 * pi / 2., 0, 1)
+    assert p.out() == 'RZ(0) 0\nRZ(pi) 1\nRZ(-pi) 2\nRZ(2*pi/3) 3\n' \
+                      'RZ(0.3490658503988659) 4\n' \
+                      'RZ(pi/8) 5\nCPHASE00(-45*pi) 0 1\n'