Skip to content

Add capability to annotate statements #34

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

Merged
merged 3 commits into from
May 11, 2023
Merged

Conversation

PhilReinhold
Copy link
Collaborator

This adds support for openqasm annotations

Annotations can be added in three ways:

  1. Program.annotate which attaches the annotation to the next statement added to the program
  2. _ClassicalVar(..., annotations=[...]), which attaches the annotation to the variable declaration statement
  3. @annotate_subroutine(...) decorator, which attaches the annotation to the subroutine declaration

Copy link
Collaborator

@jcjaskula-aws jcjaskula-aws left a comment

Choose a reason for hiding this comment

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

Minor comments so far. Believe we should discourage customers to use prog.annotate with prog.declare. I think the annotation is added to the first statement and not the others.

prog = oqpy.Program()

j = oqpy.IntVar(0, name="j", annotations=["annotation-j"])
i = oqpy.IntVar(0, name="i", annotations=["annotation-i"])
prog.annotate("first-annotation")
prog.declare([i,j])
print(prog.to_ast())

Should we defer the support for QubitDeclaration? Right now, it does not seem that we add the declaration anyway even without annotation.
Otherwise, it looks good to me.

@PhilReinhold
Copy link
Collaborator Author

Believe we should discourage customers to use prog.annotate with prog.declare. I think the annotation is added to the first statement and not the others.

I've added to the test to confirm that the annotations "work as expected" (at least how I would expect them to work) with mixing annotations between variable initialization and prog.annotate(); prog.declare().

Should we defer the support for QubitDeclaration? Right now, it does not seem that we add the declaration anyway even without annotation.

I've added code to make it work with qubit declaration as well in the same way.

Copy link
Collaborator

@jcjaskula-aws jcjaskula-aws left a comment

Choose a reason for hiding this comment

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

LGTM.

I agree that annotate() + declare should annotate the first as the parser would if we pass @my_annotation + int i=0.

@jcjaskula-aws jcjaskula-aws merged commit 7916a80 into main May 11, 2023
@jcjaskula-aws jcjaskula-aws deleted the pcrein/add-annotations branch May 11, 2023 14:50
yitchen-tim added a commit to ajberdy/oqpy that referenced this pull request May 16, 2023
* Add capability to annotate statements (openqasm#34)

* Add capability to annotate statements

* Address review comments

* style/lint fixes

* Add pragma (openqasm#35)

* Add pragma

* Return better error message

* Extend and reuse add_statement

* Add test for annotation and ifs

* Finalize if before adding annotations

* Fix finalize if with annotation and no else

* Fix typo

* Bump version to 0.2.0 (openqasm#36)

---------

Co-authored-by: Phil Reinhold <[email protected]>
Co-authored-by: Jean-Christophe Jaskula <[email protected]>
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.

2 participants