Parse OpenQASM #495

notmgsk · 2019-12-04T00:41:59Z

This (late) PR implements all functionality described in the OpenQASM spec.

The following is a somewhat complex example, demonstrating a ripple-carry adder OpenQASM program:

(qvm-app::print-classical-memory (qvm:run-program 10 (parse-qasm-string "
// quantum ripple-carry adder from Cuccaro et al, quant-ph/0410184
OPENQASM 2.0;
include \"qelib1.inc\";
gate majority a,b,c 
{ 
  cx c,b; 
  cx c,a; 
  ccx a,b,c; 
}
gate unmaj a,b,c 
{ 
  ccx a,b,c; 
  cx c,a; 
  cx a,b; 
}
qreg cin[1];
qreg a[4];
qreg b[4];
qreg cout[1];
creg ans[5];
// set input states
x a[0]; // a = 0001
x b;    // b = 1111
// add a to b, storing result in b
majority cin[0],b[0],a[0];
majority a[0],b[1],a[1];
majority a[1],b[2],a[2];
majority a[2],b[3],a[3];
cx a[3],cout[0];
unmaj a[2],b[3],a[3];
unmaj a[1],b[2],a[2];
unmaj a[0],b[1],a[1];
unmaj cin[0],b[0],a[0];
measure b[0] -> ans[0];
measure b[1] -> ans[1];
measure b[2] -> ans[2];
measure b[3] -> ans[3];
measure cout[0] -> ans[4];")))
Classical memory (low -> high indexes):
    ans: 0 0 0 0 1

There are some things to address:

There should be more error checking, e.g. making sure the sizes of registers are appropriate for the gate application.
In parse-application I explicitly define all of the gates defined in OpenQASM's qelib1.inc. This could be simplified (and instead just define the fundamental gates U and CX), but I'm not sure (because I haven't tried) whether that will Just Work.
Automatically invoke the parser from parse-quil if the first (non-comment) line is OPENQASM 2.0.

I think the test suite is generally in a good state. It covers all positive behaviours listed in the spec but it doesn't test for misuse.

stylewarning

First round of review.

src/ast.lisp

src/cl-quil.lisp

src/package.lisp

src/qasm.lisp

stylewarning · 2019-12-04T21:17:03Z

src/qasm.lisp

+  (destructuring-bind (if-toks . rest-lines)
+      tok-lines
+    (pop if-toks)
+    (q2q-check-token-type (pop if-toks) ':LEFT-PAREN)


sounds like we can maybe use token destructuring?

src/qasm.lisp

ecpeterson

Commenting to let you know that I've looked through this, & (TODOs aside) things look pretty good.

stylewarning · 2019-12-12T23:09:08Z

@notmgsk Ping when re-review is desired.

appleby

Made it about half-way through. All tuckered out. Time for Christmas cookies.

tests/qasm-tests.lisp

src/qasm.lisp

appleby · 2019-12-12T23:17:56Z

src/qasm.lisp

+  ;; a[0] -> b[0]`.
+  ;;
+  ;; TODO I wrote this while very tired. Untested.
+  (let ((ts (mapcar #'alexandria:ensure-list (remove '_ token-idents))))


feels like there should be a smattering of once-onlys but I didn't check carefully

src/qasm.lisp

appleby

There is some trailing whitespace in both qasm.lisp and qasm-tests.lisp. M-x whitespace-cleanup and/or M-x delete-trailing-whitespace to the rescue.

I don't feel like any of these are blockers per se, but I'll delay approval to maintain Code Review Best Practices®️

I'm also vaguely uneasy about *creg-names* and *qreg-names* but I don't know why. maybe I'm just hungry. time for lunch.

src/qasm.lisp

notmgsk · 2019-12-16T18:21:28Z

@appleby @stylewarning ping for re-review at you leisure

appleby

cool stuff

src/qasm.lisp

src/cl-quil.lisp

stylewarning

Some critical things, some not so critical things.

I think this PR has been a little too liberal with TODOs. On one hand they're good first issues for newcomers, on the other hand they're debt that people may not feel motivated to fix.

src/cl-quil.lisp

src/qasm.lisp

appleby

Previously approved. Just chiming in to say the latest round of changes lgtm.

src/qasm.lisp

src/cl-quil.lisp

src/qasm.lisp

jmbr

Comments (1/N).

src/cl-quil.lisp

src/qasm.lisp

jmbr

Comments (N/N).

LGTM.

jmbr · 2019-12-18T18:30:18Z

src/qasm.lisp

+(defun parse-qasm (string)
+  "Parse STRING into a PARSED-PROGRAM object, applying all transforms."
+  (let ((pp (quil::resolve-objects (parse-qasm-into-raw-program string))))
+    (setf pp (quil::transform 'quil::expand-circuits pp))


Suggestion: fuse setf forms.

jmbr · 2019-12-18T18:31:23Z

src/qasm.lisp

+         (code (parse-qasm-body string)))
+    (setf code (quil::process-includes code))


Suggestion: compose parse-qasm-body with quil::process-includes in the let* form above and discard the setf.

braised-babbage

LGTM

braised-babbage · 2019-12-18T18:48:47Z

src/qasm.lisp

+     *qreg-names*
+     *qubit-count*)))
+
+(defun parse-qasm (string)


Minor request: a keyword argument transforms which defaults a list of the three you currently use

notmgsk requested a review from a team as a code owner December 4, 2019 00:41

notmgsk added the do not merge label Dec 4, 2019

stylewarning reviewed Dec 4, 2019

View reviewed changes

ecpeterson reviewed Dec 6, 2019

View reviewed changes

appleby reviewed Dec 12, 2019

View reviewed changes

notmgsk force-pushed the feature/qasm branch 2 times, most recently from 0717f2a to b26089e Compare December 13, 2019 19:21

appleby suggested changes Dec 14, 2019

View reviewed changes

appleby approved these changes Dec 16, 2019

View reviewed changes

src/qasm.lisp Show resolved Hide resolved

appleby reviewed Dec 16, 2019

View reviewed changes

src/qasm.lisp Outdated Show resolved Hide resolved

appleby reviewed Dec 16, 2019

View reviewed changes

src/qasm.lisp Outdated Show resolved Hide resolved

notmgsk commented Dec 16, 2019

View reviewed changes

src/cl-quil.lisp Outdated Show resolved Hide resolved

rigettizach reviewed Dec 17, 2019

View reviewed changes

src/cl-quil.lisp Outdated Show resolved Hide resolved

stylewarning requested changes Dec 17, 2019

View reviewed changes

notmgsk added 14 commits December 17, 2019 16:43

OpenQASM parsing

12d32ff

fix if

d1caa9a

fixes

cb59a46

Remove use-package and fully qualify symbol

1e6743d

Remove some errant code

aad691a

remove whitespace

2266355

Don't error if lexical-context is nil

e41063b

rename and export parse-qasm

de2ec94

remove unused code

99415d2

Use defvar rather than defparameter

47507a2

Use cl-quil's token stuff

4e4f62e

use defvar rather than defparameter

83379b9

qubit is exported

e6ad31d

Address PR comms

79bcf64

notmgsk added 18 commits December 17, 2019 16:43

docstr for build-u-gate

4bc26e3

Use values

31a8091

append vs cons

e283ec6

functional programming is good

324d431

a: nickname

c3bbc6c

Delete trailing whitespace

f1a5603

Fix docstr

0e70461

todo

18cc508

Fix JUMPs

9c532a1

fix size comparison

1185c1b

Modify PARSE-QUIL to accept/parse OpenQASM

f20fa68

Fix and test %check-for-qasm-header

724f895

clean

946779a

use xach's suggestion

abb986b

Add qasm parsing to quilc

2068132

match whitespace-only lines

1c26db2

pr feedback

73c8a8f

safe resolve

1fce003

notmgsk force-pushed the feature/qasm branch from 8c3e657 to 1fce003 Compare December 18, 2019 00:51

notmgsk added 2 commits December 17, 2019 17:18

safer resolve

b088363

pr comments

56cf57f

appleby reviewed Dec 18, 2019

View reviewed changes

src/qasm.lisp Show resolved Hide resolved

src/cl-quil.lisp Show resolved Hide resolved

src/qasm.lisp Show resolved Hide resolved

jmbr reviewed Dec 18, 2019

View reviewed changes

src/cl-quil.lisp Show resolved Hide resolved

src/qasm.lisp Show resolved Hide resolved

src/qasm.lisp Show resolved Hide resolved

src/qasm.lisp Show resolved Hide resolved

src/qasm.lisp Show resolved Hide resolved

src/qasm.lisp Show resolved Hide resolved

jmbr approved these changes Dec 18, 2019

View reviewed changes

braised-babbage approved these changes Dec 18, 2019

View reviewed changes

notmgsk removed the do not merge label Dec 18, 2019

stylewarning merged commit 8ae5c87 into master Dec 18, 2019

stylewarning deleted the feature/qasm branch December 18, 2019 19:24

notmgsk mentioned this pull request Dec 18, 2019

Bump version to 1.15.0 #504

Merged

ecpeterson mentioned this pull request Dec 30, 2019

Incorporate benchmarks #465

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parse OpenQASM #495

Parse OpenQASM #495

notmgsk commented Dec 4, 2019 •

edited

Loading

stylewarning left a comment

stylewarning Dec 4, 2019

ecpeterson left a comment

stylewarning commented Dec 12, 2019

appleby left a comment

appleby Dec 12, 2019

appleby left a comment

notmgsk commented Dec 16, 2019

appleby left a comment

stylewarning left a comment

appleby left a comment

jmbr left a comment

jmbr left a comment

jmbr Dec 18, 2019

jmbr Dec 18, 2019

braised-babbage left a comment

braised-babbage Dec 18, 2019

		(code (parse-qasm-body string)))
		(setf code (quil::process-includes code))

Parse OpenQASM #495

Parse OpenQASM #495

Conversation

notmgsk commented Dec 4, 2019 • edited Loading

stylewarning left a comment

Choose a reason for hiding this comment

stylewarning Dec 4, 2019

Choose a reason for hiding this comment

ecpeterson left a comment

Choose a reason for hiding this comment

stylewarning commented Dec 12, 2019

appleby left a comment

Choose a reason for hiding this comment

appleby Dec 12, 2019

Choose a reason for hiding this comment

appleby left a comment

Choose a reason for hiding this comment

notmgsk commented Dec 16, 2019

appleby left a comment

Choose a reason for hiding this comment

stylewarning left a comment

Choose a reason for hiding this comment

appleby left a comment

Choose a reason for hiding this comment

jmbr left a comment

Choose a reason for hiding this comment

jmbr left a comment

Choose a reason for hiding this comment

jmbr Dec 18, 2019

Choose a reason for hiding this comment

jmbr Dec 18, 2019

Choose a reason for hiding this comment

braised-babbage left a comment

Choose a reason for hiding this comment

braised-babbage Dec 18, 2019

Choose a reason for hiding this comment

notmgsk commented Dec 4, 2019 •

edited

Loading