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

Add some more tests for DEFEXPI functionality #501

Merged
merged 3 commits into from
Jan 2, 2020
Merged

Conversation

ecpeterson
Copy link
Contributor

@ecpeterson ecpeterson commented Dec 18, 2019

@@ -984,20 +984,22 @@ If ENSURE-VALID is T, then a memory reference such as 'foo[0]' will result in an
:arguments qubit-list))))

(defun pauli-term->matrix (term arguments parameters parameter-names)
(let* ((prefactor-fn
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering where this commit snuck off to.

@ecpeterson ecpeterson marked this pull request as ready for review January 2, 2020 16:42
@ecpeterson ecpeterson requested a review from a team as a code owner January 2, 2020 16:42
@ecpeterson
Copy link
Contributor Author

The only suggested test not added is an explicit test involving parametric-diagonal-compiler. The role of that compiler is to decompose exponentiated Pauli sums only containing Zs into CNOTs and RZs (in a way that permits the Pauli terms to have underdetermined coefficients). I'm honestly not sure what a good test would look like that would instill any more confidence in me (or provide a more isolated bug report) than test-parametric-defexpi.

Copy link
Contributor

@appleby appleby left a comment

Choose a reason for hiding this comment

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

Rubber stamping a bit here, but I am always in favor of moar tests :-)

@jmbr
Copy link
Contributor

jmbr commented Jan 2, 2020

Suggestion: test against cl-quil.clifford:exp-pauli.

(let* ((cpp (compiler-hook (parse-quil prog-text) chip))
(m (parsed-program-to-logical-matrix cpp)))
(is (quil::double= 1d0 (abs (magicl:ref m 0 0))))
(loop :for j :below (magicl:matrix-rows m)
Copy link
Member

Choose a reason for hiding this comment

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

magicl:identityp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is checking identityp up to phase.

(declare (ignorable ,@parameter-names))
,(delayed-expression-expression (pauli-term-prefactor term)))))
(apply
(compile nil `(lambda ,parameter-names
Copy link
Member

Choose a reason for hiding this comment

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

Definitely not for this commit, but we might instead simply interpret the expression instead of calling the compiler.

@stylewarning stylewarning merged commit 8da0c76 into master Jan 2, 2020
@stylewarning
Copy link
Member

Suggestion: test against cl-quil.clifford:exp-pauli.

i think this is a good suggestion. Could @jmbr or @ecpeterson make a "good-first-issue" for this?

@stylewarning stylewarning deleted the tests/defexpi branch January 2, 2020 21:45
@ecpeterson
Copy link
Contributor Author

I will.

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.

5 participants