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

Bugfix: rework some of the standard gates init logic to accommodate seq gates #809

Merged

Conversation

Spin1Half
Copy link
Contributor

@Spin1Half Spin1Half commented Apr 6, 2022

It was discovered when fiddling with some of the std gate definitions that the recent sequence gate definition validation/resolution aren't fully integrated for sequence gate definitions added to this file

@Spin1Half Spin1Half closed this Apr 7, 2022
@Spin1Half Spin1Half reopened this Apr 7, 2022
@Spin1Half Spin1Half changed the title Rework some of the standard gates init logic to accommodate seq gates Bugfix: rework some of the standard gates init logic to accommodate seq gates Apr 8, 2022
src/initialize-standard-gates.lisp Outdated Show resolved Hide resolved
src/initialize-standard-gates.lisp Outdated Show resolved Hide resolved
src/initialize-standard-gates.lisp Outdated Show resolved Hide resolved
src/gates.lisp Outdated Show resolved Hide resolved
src/gates.lisp Outdated Show resolved Hide resolved
src/gates.lisp Show resolved Hide resolved
@Spin1Half Spin1Half force-pushed the bugfix-resolve-validate-seq-std-gates branch from 07e47ea to c5601f3 Compare April 8, 2022 23:36
@Spin1Half Spin1Half closed this Apr 11, 2022
@Spin1Half Spin1Half reopened this Apr 11, 2022
@Spin1Half Spin1Half marked this pull request as draft April 11, 2022 19:08
@Spin1Half Spin1Half force-pushed the bugfix-resolve-validate-seq-std-gates branch from c5601f3 to b71ead8 Compare April 11, 2022 20:59
@Spin1Half Spin1Half marked this pull request as ready for review April 21, 2022 21:26
@Spin1Half
Copy link
Contributor Author

This was in draft because of issues I was encountering for an unrelated reason, I will address these issues in a separate PR

(parsed-program-gate-definitions parsed-program)))

(unless (boundp' **default-gate-definitions**)
(eval-when (:compile-toplevel :load-toplevel :execute)
Copy link
Member

Choose a reason for hiding this comment

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

I think this eval-when needs to be on the outside

also please write a comment above this block of code to explain what it's doing anx why it's here. It's unusual to see toplevel code getting evaluated like this.

Lastly, I would wrap this stuff in a function itself, and do the eval-when stuff around a call to it.

(eval-when (...)
  (defun read-standard...)

  (defun initialize-standard-gate-definitions...)

  (initialize-standard-gate-definitions)

)  ; eval-when

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented the structural changes.

Did my best to explain what was going on, but I don't know that I was as informative as I could be

@Spin1Half Spin1Half force-pushed the bugfix-resolve-validate-seq-std-gates branch from b71ead8 to 3ba4154 Compare April 22, 2022 00:46
@Spin1Half Spin1Half closed this Apr 22, 2022
@Spin1Half Spin1Half reopened this Apr 22, 2022
@Spin1Half Spin1Half force-pushed the bugfix-resolve-validate-seq-std-gates branch from 3ba4154 to 09d7ad7 Compare April 22, 2022 22:40
(setf (gethash (gate-definition-name gate-def) table)
gate-def)))))))

(initialize-standard-gates));eval-when
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(initialize-standard-gates));eval-when
(initialize-standard-gates)
) ; eval-when

@Spin1Half Spin1Half force-pushed the bugfix-resolve-validate-seq-std-gates branch from 09d7ad7 to af48e69 Compare April 22, 2022 22:44
@Spin1Half Spin1Half force-pushed the bugfix-resolve-validate-seq-std-gates branch from af48e69 to 19f2863 Compare April 22, 2022 22:45
@stylewarning stylewarning merged commit ae56124 into quil-lang:master Apr 22, 2022
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

2 participants