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 support for arguments & return types to defcals #20

Merged
merged 7 commits into from
Nov 16, 2022

Conversation

jcjaskula-aws
Copy link
Collaborator

@jcjaskula-aws jcjaskula-aws commented Nov 10, 2022

The PR adds support for arguments and return type in defcal.

Arguments are provided via a list of ASTConvertible. Classical variables are passed to the defcal block through the context manager.

Return types are ast.ClassicalType or OQpy types.

The keys of the self.defcal dict have been modified to include a tuple of arguments.

Closes #7

@jcjaskula-aws jcjaskula-aws force-pushed the jcjaskula-aws/add_defcal_args branch from e21617e to 326c330 Compare November 10, 2022 21:04
program: Program,
qubits: Union[Qubit, list[Qubit]],
name: str,
arguments: Optional[list[Union[_ClassicalVar, str]]] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

The behavior for fixed parameters is not quite right here, in that if we reparse the generated openqasm it won't match the AST we store. The issue is assuming the string is an identifier. In fact the value you pass in your examples is not an identifier, but instead the binary expression pi/2.

I think the type here should be Optional[list[AstConvertible]] and shouldn't take arbitrary strings.

We may also want to define a module constant oqpy.pi = AngleType(name="pi", needs_declaration=False). so that the user need not define a pi constant for themselves

Copy link
Collaborator Author

@jcjaskula-aws jcjaskula-aws Nov 12, 2022

Choose a reason for hiding this comment

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

It won't with the reference parser indeed. I'll see if I can be satisfied with OQPyExpression.

Copy link
Collaborator

@PhilReinhold PhilReinhold left a comment

Choose a reason for hiding this comment

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

Small comment but otherwise LGTM!

oqpy/classical_types.py Show resolved Hide resolved
@karalekas karalekas changed the title Add arguments to defcals Add support for arguments & return types to defcals Nov 13, 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.

Support classical arguments and return types for defcals
2 participants