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

Parse and implement class for permutation gates #891

Merged
merged 9 commits into from May 20, 2019

Conversation

notmgsk
Copy link
Contributor

@notmgsk notmgsk commented Apr 23, 2019

Introduces a class DefPermutationGate which subclasses DefGate, so that gates can be defined using a permutation matrix: cnot = DefPermutationGate([0, 1, 3, 2]). Normal DefGate stuff works, like DefGate.num_args() and DefGate.get_constructor().

Also update the parser to accept permutations:

In [11]: p = Program("""DEFGATE CCNOT AS PERMUTATION:
    ...:     0, 1, 2, 3, 4, 5, 7, 6
    ...: CCNOT 3 2 1""")

In [12]: p._defined_gates
Out[12]: [<pyquil.quilbase.DefPermutationGate at 0x121a63c50>]

In [13]: p._defined_gates[0]
Out[13]: <pyquil.quilbase.DefPermutationGate at 0x121a63c50>

In [14]: p._defined_gates[0].get_constructor()
Out[14]: <function pyquil.quilbase.DefGate.get_constructor.<locals>.<lambda>(*qubits)>

In [15]: p._defined_gates[0].get_constructor()(0, 1, 2)
Out[15]: <Gate CCNOT 0 1 2>

In [16]: p._defined_gates[0].get_constructor()(0, 1)
Out[16]: <Gate CCNOT 0 1>

In [17]: p._defined_gates[0].num_args()
Out[17]: 3

@ecpeterson
Copy link
Contributor

I'm a little surprised at In [16]. It doesn't throw a fit when you pass it too few qubit arguments?

Copy link
Contributor

@ecpeterson ecpeterson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remarkably painless, lgtm

@notmgsk
Copy link
Contributor Author

notmgsk commented Apr 23, 2019

@ecpeterson DefGate does not throw an error, either. I can change that behaviour for both if you'd prefer.

pyQuil is a bit messy in this regard. Some places make sanity checks, others don't.

@ecpeterson
Copy link
Contributor

I think I'd like that sanity check to be made when possible. I can't imagine anyone is relying on this bad behavior of generating illegal Quil. I suppose it doesn't have to be in this PR, though.

pyquil/_parser/PyQuilListener.py Outdated Show resolved Hide resolved
pyquil/_parser/PyQuilListener.py Outdated Show resolved Hide resolved
pyquil/quilbase.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ecpeterson ecpeterson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still looks fine to me.

Out of curiosity: when a user writes a Quil program with a bad permutation (like 0, 1, 1, 3), who eventually catches this error and what error do they signal?

@notmgsk
Copy link
Contributor Author

notmgsk commented May 15, 2019

Still looks fine to me.

Out of curiosity: when a user writes a Quil program with a bad permutation (like 0, 1, 1, 3), who eventually catches this error and what error do they signal?

After this is merged quil-lang/quilc#234, the above is caught at parse time in quilc.

rpcq._utils.RPCError: At line 1: Permutation gate entries do not represent a square matrix

@ecpeterson
Copy link
Contributor

Cool. Still curious: what happens without this new quilc guard? Does tweedledum throw an error? Does it just give garbage results? (As an aside, I think the Quil spec promises only garbage-in-garbage-out with regards to non-unitary DEFGATEs, and we only deign to throw explicit errors as it suits us.)

@notmgsk
Copy link
Contributor Author

notmgsk commented May 15, 2019

@ecpeterson something unhelpful like

The value
  #C(#<DOUBLE-FLOAT quiet NaN> 1.5707963267948966d0)
is not of type
  DOUBLE-FLOAT

Copy link
Contributor

@karalekas karalekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed @notmgsk this change is not backwards compatible with older versions of quilc (<= 1.7.2 I believe), and so we should make the extra effort to warn (and dare I say prevent) people from using this feature with those versions of quilc.

Copy link
Contributor

@stylewarning stylewarning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one little change, otherwise i think this is Good 2 Go (TM).

docs/source/basics.rst Outdated Show resolved Hide resolved
Co-Authored-By: Robert Smith <robert@stylewarning.com>
@karalekas karalekas dismissed stylewarning’s stale review May 20, 2019 23:33

The change requested above has been made

@karalekas karalekas merged commit be212d0 into master May 20, 2019
@karalekas karalekas deleted the feature/permutation-gates branch May 20, 2019 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants